ringhash: fix bug where ring hash can be stuck in transient failure despite having available endpoints

[atollena]

See #7363 for a problem description. Added
test cases reproduce the error without the fix, but not reliably, since the ring
may be constructed in a way where we don't get stuck. However running the test
multiple times definitely end up triggering the problem.

The solution is to keep a separate list of available addresses, and when there are no
picks and we trigger connection attempts, try them one at a time.

Fixes #7363.

RELEASE NOTES:

• ringhash: when used with multiple EDS priorities, fix bug that could prevent a higher priority from recovering from transient failure.

[atollena]

Depends on #7334.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.98%. Comparing base (ced812e) to head (67181a1).
Report is 20 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7364      +/-   ##
==========================================
+ Coverage   81.58%   81.98%   +0.40%     
==========================================
  Files         358      360       +2     
  Lines       27388    27528     +140     
==========================================
+ Hits        22345    22570     +225     
+ Misses       3828     3773      -55     
+ Partials     1215     1185      -30     

Files	Coverage Δ
internal/testutils/blocking_context_dialer.go	100.00% <100.00%> (ø)
xds/internal/balancer/ringhash/picker.go	86.79% <ø> (+3.21%)	⬆️
xds/internal/balancer/ringhash/ringhash.go	88.88% <100.00%> (+0.17%)	⬆️

... and 40 files with indirect coverage changes

[arvindbr8]

One comment, could you please take a look?

Overall looks good to me. One small bummer is that we are adding yet another field to store the subconn(s). I was thinking hard if making AddressMap an ordered Map (by morphing Keys() and Values()) would help. But then indexing the map with numbers becomes an issue.

[arvindbr8]

When we publish relnotes, it would probably help users with some more additional context. for eg: this bug could have effected you if ___

[atollena]

Thanks for the review.

One comment, could you please take a look?

Overall looks good to me. One small bummer is that we are adding yet another field to store the subconn(s). I was thinking hard if making AddressMap an ordered Map (by morphing Keys() and Values()) would help. But then indexing the map with numbers becomes an issue.

I am not sure what you suggest exactly. Do you mean that we could use the existing AddressMap, and turn it into an array by sorting it by key, for example?

[arvindbr8]

Do you mean that we could use the existing AddressMap, and turn it into an array by sorting it by key, for example?

yes, but we dont have to block merge on that. LGTM! Thanks @atollena for taking care of this

[easwars]

Change is LGTM. A bunch of minor comments on the tests though.

[easwars]

Thanks @atollena for handling all the comments and for your continued contributions.