endpointsharding: shuffle endpoint order before updating children #8438
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8438 +/- ##
==========================================
- Coverage 82.34% 82.32% -0.03%
==========================================
Files 414 414
Lines 40411 40433 +22
==========================================
+ Hits 33276 33286 +10
- Misses 5771 5781 +10
- Partials 1364 1366 +2
🚀 New features to boost your workflow:
|
|
|
||
| // rotateEndpoints returns a slice of all the input endpoints rotated a random | ||
| // amount. | ||
| func rotateEndpoints(es []resolver.Endpoint) []resolver.Endpoint { |
There was a problem hiding this comment.
It looks like this actually changes the order of the address list, rather than simply starting to connect from a random index. But maybe that amounts to the same thing, since each new picker starts from a random index anyway in RR?
There was a problem hiding this comment.
This function rotates the slice a random amount -- it puts all elements after the random point at the start of a new slice, and the elements before it at the end.
| } | ||
|
|
||
| // The sc# corresponding to addr3 is not known due to possible randomization | ||
| // in connection order in RR. Check the address instead of sc and set |
There was a problem hiding this comment.
If I understand correctly , we have changed sc3 to correspond to address 3 in the above change. I am not sure what this comment accomplishes?
There was a problem hiding this comment.
Oops, I changed the way I did this and left this comment from the old way behind. Removed
3 participants
See b/412468630, and esp. comment 46. Essentially, if the connection order is not randomized then the first address gets hammered with all the client's initial requests when traffic is bursty. If multiple clients are all using the same order and many start up around the same time, then the first address can see that problem magnified even further. Randomizing at least spreads the load between the clients, even if it still results in one address getting hammered at startup.
The diffs are much smaller than it shows. Basically, the existing
e.s._test.gowas moved to..._ext_test.goand the only new code is in..._test.go._ext_test.gois completely unchanged from the old_test.go. Weightedtarget's tests needed changes because they assumed the order in which subchannels were created matched the order in which endpoints were provided.RELEASE NOTES: