8000 [ISSUE #9013] enhance grpc client by karsonto · Pull Request #9017 · alibaba/nacos · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[ISSUE #9013] enhance grpc client #9017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 29, 2022

Conversation

karsonto
Copy link
Contributor

Fix the problem for #9013
Add some custom config in grpc config
e.g. nacos.client.grpc.pool.alive , nacos.client.grpc.timeout

@codecov-commenter
Copy link
codecov-commenter commented Aug 23, 2022

Codecov Report

Merging #9017 (67e3c42) into develop (3c8c6a7) will increase coverage by 0.02%.
The diff coverage is 75.67%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #9017      +/-   ##
=============================================
+ Coverage      43.65%   43.67%   +0.02%     
- Complexity      4763     4766       +3     
=============================================
  Files            912      912              
  Lines          32569    32591      +22     
  Branches        3761     3762       +1     
=============================================
+ Hits           14218    14235      +17     
- Misses         16951    16956       +5     
  Partials        1400     1400              
Impacted Files Coverage Δ
...ba/nacos/common/remote/client/grpc/GrpcClient.java 28.28% <75.67%> (+12.90%) ⬆️
...onsistency/persistent/ClusterVersionJudgement.java 53.48% <0.00%> (-13.96%) ⬇️
...os/client/auth/ram/identify/CredentialService.java 78.84% <0.00%> (-3.85%) ⬇️
.../core/v2/event/publisher/NamingEventPublisher.java 76.25% <0.00%> (+1.25%) ⬆️
...alibaba/nacos/core/remote/control/TpsRecorder.java 75.67% <0.00%> (+1.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c8c6a7...67e3c42. Read the comment docs.


private final int queueSize = 10000;

private final Long keepAlive = 10L;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keepAlive and defaultTimeOut is only used by String.valueOf should we define them as String directly?

And the constants I think should as static final

one more about the queueSize should we make it can config also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KomachiSion 按你建议修改了一下代码,加载配置顺序为JVM 参数 --> 构造器传入Properties ---> Default 配置

@KomachiSion
Copy link
Collaborator

关于GrpcClient的healthcheck, 当前只是等待了一个超时时间(默认3s), 只要一次失败就会断开链接,尝试重连,其实过敏了, 单纯的只是增加超时时间其实也不是最优解, 最优的方法是增加一个重试次数,达到一定次数失败之后才认为链接断开,去重连。

这部分逻辑你可以新提一个PR优化一下。

@KomachiSion KomachiSion merged commit 9819820 into alibaba:develop Aug 29, 2022
@KomachiSion KomachiSion added the kind/enhancement Category issues or prs related to enhancement. label Aug 29, 2022
@KomachiSion KomachiSion added this to the 2.1.2 milestone Aug 29, 2022
li-xiao-shuang pushed a commit that referenced this pull request Aug 31, 2022
* Add exception detail stacks when connect to server failed.

* [ISSUE#8653]Fix health check bug (#8639)

* fix health bug.

* fix health bug.

* fix health bug.

* test update

* format code

* 注释

* check service exist

* check service exist

* test fail

* user group()

* add unit test.

* add unit test.

* add unit test.

* add unit test.

* add unit test.

* add unit test.

* Add comment for config aspect.

* [ISSUE#8481]Unified TRACE capacity building  (#8805)

* [ISSUE#8481]Unified TRACE capacity building

* [ISSUE#8481]Unified TRACE capacity building
1. Event type add to traceEvent.
2. The event about instance add instancePort.
3. Add health check type to healthChangeEvent.
4. Other fixup.

* Add Generic Type for Simple Cache Map (#8812)

* import config file auth fail (#8844)

* import config file auth fail

* run ci

* run ci2

* Feat/calculate ip count when use batchregister (#8858)

Feat/calculate ip count when use batchregister

* [ISSUE #8880] add a request exception handler (#8881)

* [ISSUE #8880] add a request exception handler

Close #8880

* [ISSUE #8880] retry ci

* Upgrade to 2.1.1 (#8899)

* [ISSUE#8910] bug fix calcu ipCount (#8911)

* bug fix calcu ipCount

* update logic

* judge endpoint (#8933)

* [ISSUE #8925] fix that the value of hasQueryString is always false (#8927)

Close #8925

* [ISSUES#8905]simplify some code (#8908)

* refactor: 💡 update console-ui theme (#8951)

✅ Closes: #8950

* Update IoUtils.java (#8948)

An IOException has been catch,so throws IOException Is invalid

* [ISSUE #8931] Fix bug :BatchInstanceData cannot Serializable question (#8940)

* [fix] fix BatchInstanceData cannot Serializable question

* fix update serialVersionUID auto generate

* [ISSUE #8925] fix that the replaceAll operation is invalid (#8929)

Close #8925

* fix #8947. (#8959)

* judge endpoint

* add some junit test (#8966)

* add some junit test

* fix some junit

* add license

* optimize: internationalization of product description content on landing pages (#8956)

* Improve code specifications. (#8965)

Flip the call to equals method to avoid null pointer exception.

* fix LookupFactoryTest  ut / ci (#8985)

* fix ci

* run ci

* check cla report

* check style

* add some auth plugin test (#8988)

* add some auth plugin test

* ru ci

* remove namespaceShowName uniqueness check (#8989)

* add unit test for ReflectUtils (#8999)

* add unit test for ReflectUtils

* reformat code

* [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common] (#8997)

* [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common]

* [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common]

* [ISSUE #8993]Close input stream and output stream by try with resource[nacos-common]

* add some utils UT (#9006)

add some utils UT

* Fix grpc push data not filter by cluster. (#9019)

* [#344] rename initialization sql files (#8969)

* [ISSUE #9014] performance optimization--refacte TpsMonitorPoint (#9021)

* performance optimization

* fix check style

* fix test unit

* fix test unit for `getTimeFormatOfSecond` method.

* fix unit test  for LimiterTest.

* fix error usage case for asserEquals method;eliminate interactions between test methods

* Fix spelling mistakes. (#9026)

* [ISSUE #9013] enhance grpc client (#9017)

* fix issue #9013

* reformat code style

* reformat code style

* reformat code style

* reformat code style

* reformat code style

* reformat code style

* reformat code style

* change config name

* do some enhance

* add queue size to configProperties

* reformat code style

* Change configuration load order

* Fix ServerListMgr is not shutdown. (#9048)

* set server.error.include-message to ALWAYS (#9045)

* [ISSUE #8934] add header back after convert request to jsonBytes (#8935)

* [ISSUE #9023] fix corner case config dataId 'cipher-' that cannot create (#9024)

* [ISSUE #9023] fix corner case config dataId 'cipher-' that cannot create

* [ISSUE #9023] remove unused import

* [ISSUE #9023] PMD fix

* [ISSUE #9023] add comments

* [ISSUE #9023] should return first token instead of any

* [ISSUE #9023] add more EncryptionHandler ut

* [ISSUE #9023] fix style

* [ISSUE #9023] fix style, keep indentation in blank line

Co-authored-by: KomachiSion <xiweng.yy@alibaba-inc.com>
Co-authored-by: 胡俊 <510830970@qq.com>
Co-authored-by: MajorHe1 <53824889+MajorHe1@users.noreply.github.com>
Co-authored-by: GuiShao <zhupeng.zhu@alibaba-inc.com>
Co-authored-by: ZhangShenao <15201440436@163.com>
Co-authored-by: chenhao26 <35129699+chenhao26-nineteen@users.noreply.github.com>
Co-authored-by: onewe <admin@onew.me>
Co-authored-by: 胡俊 <hujun3@xiaomi.com>
Co-authored-by: liushaoyu-soft <455028117@qq.com>
Co-authored-by: heimanba <371510756@qq.com>
Co-authored-by: 无处不在 <42382506+wuchubuzai2018@users.noreply.github.com>
Co-authored-by: Weizhan Yun <yunweizhan@gmail.com>
Co-authored-by: liaochuntao <liaochuntao@live.com>
Co-authored-by: @xiaochangbai <704566072@qq.com>
Co-authored-by: shiherua <56618514+shiherua@users.noreply.github.com>
Co-authored-by: Karson <karsontao@hotmail.com>
Co-authored-by: yjqg6666 <jinking.this@gmail.com>
Co-authored-by: 阿魁 <670569467@qq.com>
Co-authored-by: wuwen <wuwen.55@aliyun.com>
Co-authored-by: hutt <200714916@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0