-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: block RPCs early until the resolver has returned addresses #2409
Conversation
ff2d607
to
c53deec
Compare
c53deec
to
cceeb34
Compare
clientconn.go
Outdated
@@ -402,6 +403,10 @@ type ClientConn struct { | |||
balancerWrapper *ccBalancerWrapper | |||
retryThrottler atomic.Value | |||
|
|||
resolvedAddrsOnce sync.Once | |||
resolvedAddrs chan struct{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: rename this field? This sounds like a []Address
to me.
Updated to use a |
clientconn.go
Outdated
@@ -402,6 +404,8 @@ type ClientConn struct { | |||
balancerWrapper *ccBalancerWrapper | |||
retryThrottler atomic.Value | |||
|
|||
resolvedOnce *grpcsync.Event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name... This looks like a sync.Once
...
How about resolvedEvent
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
firstResolveEvent
?
This allows the initial RPC(s) an opportunity to apply settings from the service config; without this change we would still block, but only after observing the current service config settings.