Skip to content

Conversation

harbourlga
Copy link
Contributor

Description (what this PR does / why we need it):

Which issue(s) this PR fixes (resolves / be part of):

Other special notes for the reviewers:

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jan 22, 2024
@harbourlga
Copy link
Contributor Author

When the idle mode ends, watch again does not continue to obtain the instance list of the service through consul.

@shenqidebaozi
Copy link
Member

shenqidebaozi commented Jan 30, 2024

Can it be fixed when multiple clients are created for the same service? For example, if a client is idle, will b client also be unable to update the instance due to Watcher shutdown?

func (w *watcher) Stop() error {

@harbourlga
Copy link
Contributor Author

Can it be fixed when multiple clients are created for the same service? For example, if a client is idle, will b client also be unable to update the instance due to Watcher shutdown?

func (w *watcher) Stop() error {

If you are talking about multiple grpc clients, then each client will build a different resolver to watch name resolution updates. Each resolver has its own context to control the shutdown of the Watcher. Therefore, when client a becomes idle, client b will not close the watcher.

When creating the grpc client, a resolver will be built:
https://github.com/grpc/grpc-go/blob/02858ee5064040458f7e3e04e10765a62814c50b/resolver_wrapper.go#L80

ctx, cancel := context.WithCancel(context.Background())

w, err := b.discoverer.Watch(ctx, strings.TrimPrefix(target.URL.Path, "/"))

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Feb 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.72%. Comparing base (9106991) to head (658b5af).
Report is 15 commits behind head on main.

❗ Current head 658b5af differs from pull request most recent head 7437ac0. Consider uploading reports for the commit 7437ac0 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3162      +/-   ##
==========================================
+ Coverage   84.62%   84.72%   +0.10%     
==========================================
  Files          88       88              
  Lines        3993     3993              
==========================================
+ Hits         3379     3383       +4     
+ Misses        440      438       -2     
+ Partials      174      172       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…est instance of the service

* Add test cases

* Delete serviceSet when serviceSet has no watcher

* The context of resolve is controlled independently

* resolve use set context

* Remove test declare and do not use it

* gofmt

* golangci-lint fix
@dosubot dosubot bot added the LGTM label Mar 19, 2024
@shenqidebaozi shenqidebaozi changed the title fix: 当grpc结束闲置模式的时候,需要继续去获取服务的最新实例 fix(consul): unable to recreate after canceled the consul watch Mar 19, 2024
@daemon365 daemon365 merged commit 1fdaabb into go-kratos:main Mar 20, 2024
@harbourlga harbourlga deleted the fix-consul-grpcidle branch July 4, 2024 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[consul Registry]: When grpc enters idle mode, “last connection error” occurs on the client and cannot be recovered
4 participants