Skip to content
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

Client pool enhance #1119

Merged
merged 10 commits into from
Apr 25, 2021
Merged

Client pool enhance #1119

merged 10 commits into from
Apr 25, 2021

Conversation

wenxuwan
Copy link
Member

@wenxuwan wenxuwan commented Mar 31, 2021

What this PR does:

  1. gettyRPCClientPool 理论上是完全可以删除的,但现在保留,只是gettyRPCClient 不再用切片,而是一个单例

Which issue(s) this PR fixes:

  1. 现在底层的tcp连接数量其实是由gettyRPCClient 来控制的。
  2. gettyRPCClientPool 的数量配置显的有点多余,本身只会加大锁的复杂度。和用户配置的复杂度
  3. 在dubb-go刚启动的时候,如果大量并发请求,newGettyRPCClientConn 由于没有锁的保护,会打开大量的tcp连接。

@wenxuwan
Copy link
Member Author

QA:

  1. 如果gettyRPCClientPool 没用的话,建议直接配置文件里面删掉对其的一些配置,比如size。
  2. 过期时间有无必要?现在的代码会判断gettyRPCClient 有没有过期,按照我理解的话,tcp底层应该是长连接的,没必要进行惰性删除。

@AlexStocks AlexStocks changed the base branch from master to 3.0 March 31, 2021 05:15
@wenxuwan wenxuwan marked this pull request as ready for review April 10, 2021 13:23
@AlexStocks
Copy link
Contributor

AlexStocks commented Apr 11, 2021

”gettyRPCClientPool 理论上是完全可以删除的“ 如是,那就删掉吧。

@codecov-io
Copy link

Codecov Report

Merging #1119 (02d9190) into 3.0 (a347a04) will decrease coverage by 2.24%.
The diff coverage is 61.64%.

❗ Current head 02d9190 differs from pull request most recent head 47410ff. Consider uploading reports for the commit 47410ff to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##              3.0    #1119      +/-   ##
==========================================
- Coverage   60.03%   57.79%   -2.25%     
==========================================
  Files         260      252       -8     
  Lines       12855    12423     -432     
==========================================
- Hits         7718     7180     -538     
- Misses       4174     4341     +167     
+ Partials      963      902      -61     
Impacted Files Coverage Δ
cluster/cluster_impl/available_cluster.go 100.00% <ø> (ø)
cluster/cluster_impl/broadcast_cluster_invoker.go 76.47% <ø> (ø)
cluster/cluster_impl/failfast_cluster_invoker.go 66.66% <ø> (ø)
cluster/cluster_impl/mock_cluster.go 0.00% <0.00%> (ø)
cluster/loadbalance/random.go 100.00% <ø> (ø)
common/extension/cluster.go 0.00% <ø> (ø)
common/extension/config_center.go 0.00% <ø> (ø)
common/extension/config_center_factory.go 0.00% <ø> (ø)
common/extension/config_post_processor.go 0.00% <0.00%> (ø)
common/extension/configurator.go 0.00% <ø> (ø)
... and 201 more

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 95c99f0...47410ff. Read the comment docs.

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2021

Codecov Report

❗ No coverage uploaded for pull request base (3.0@0bd7945). Click here to learn what that means.
The diff coverage is 58.85%.

Impacted file tree graph

@@          Coverage Diff           @@
##             3.0    #1119   +/-   ##
======================================
  Coverage       ?   57.48%           
======================================
  Files          ?      252           
  Lines          ?    12439           
  Branches       ?        0           
======================================
  Hits           ?     7151           
  Misses         ?     4391           
  Partials       ?      897           
Impacted Files Coverage Δ
cluster/cluster_impl/mock_cluster.go 0.00% <0.00%> (ø)
common/extension/router_factory.go 0.00% <ø> (ø)
common/proxy/proxy_factory/default.go 13.55% <0.00%> (ø)
common/url.go 60.00% <0.00%> (ø)
config/base_config.go 60.00% <0.00%> (ø)
config/service_config.go 57.51% <0.00%> (ø)
config_center/nacos/impl.go 70.83% <ø> (ø)
config_center/zookeeper/impl.go 69.73% <ø> (ø)
filter/filter_impl/graceful_shutdown_filter.go 76.00% <0.00%> (ø)
metadata/definition/definition.go 63.63% <0.00%> (ø)
... and 73 more

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 0bd7945...d3a9a2d. Read the comment docs.

@AlexStocks
Copy link
Contributor

如果代码改完,还请帮忙把 dubbo-go 和 dubbo-go-samples 中相关无用参数清理下。thx。

@wenxuwan
Copy link
Member Author

wenxuwan commented Apr 18, 2021

如果代码改完,还请帮忙把 dubbo-go 和 dubbo-go-samples 中相关无用参数清理下。thx。

dubbo-go里面已经清理了,samples的等dubbo-go这边合并进去再来改

@AlexStocks AlexStocks merged commit e9f16c8 into apache:3.0 Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants