balancer: automatically stop producers on subchannel state changes #7663
Conversation
|
|
||
| func (w *weightedSubConn) updateORCAListener(cfg *lbConfig) { | ||
| if w.stopORCAListener != nil { |
There was a problem hiding this comment.
Given that the subchannel now takes care of closing the producer, do you think it makes sense to simplify things here by getting rid of the stopORCAListener field?
There was a problem hiding this comment.
No, I wanted to do that, but we still need to restart the listener when the config changes.
producer_ext_test.go
Outdated
There was a problem hiding this comment.
Why are these tests no longer required?
There was a problem hiding this comment.
There's no more races like these since we don't support starting a producer before READY.
I did think of an interesting test case, which I will add (if I can remember it again :P).
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7663 +/- ##
==========================================
+ Coverage 81.86% 81.94% +0.08%
==========================================
Files 361 361
Lines 27821 27818 -3
==========================================
+ Hits 22775 22796 +21
+ Misses 3847 3837 -10
+ Partials 1199 1185 -14
|
dfawley
added
Type: Behavior Change
clientconn.go
Outdated
| // getTransport waits until the addrconn is ready and returns the transport. | ||
| // If the context expires first, returns an appropriate status. If the | ||
| // addrConn is stopped first, returns an Unavailable status error. |
There was a problem hiding this comment.
This comment needs updating now.
There was a problem hiding this comment.
I'll do one better and just delete the function.
This change tweaks the
ProducerAPI as follows:Producerson aSubConnautomatically have theirclose()method called on every state change.GetOrBuildProduceris documented as only to be used on a READYSubConn. Using it on a non-READYSubConnwon't error, but isn't supported.I believe this fixes any possible remaining races with the
Producerinterface. #7651 fixed the issue of producers making calls before a subchannel went READY, but it was still possible for listeners to be invoked with updates after a subchannel disconnected and went to IDLE. Because the channel now callsclose()on the producer before delivering any state change update, and becausecloseis synchronous (or should be, according to the documentation), it should be impossible for the listener to receive a call after the subchannel disconnects.RELEASE NOTES: