xds: change ring_hash LB aggregation rule to handles transient_failures #9084
Conversation
| if (!connectionAttemptIterator.hasNext()) { | ||
| connectionAttemptIterator = subchannels.values().iterator(); | ||
| } | ||
| if (connectionAttemptIterator.hasNext()) { |
There was a problem hiding this comment.
This can be an else clause.
There was a problem hiding this comment.
no we do this regardless of resetting the iterator.
There was a problem hiding this comment.
Is there a reason to have this condition at all? The only way it would produce results is if subchannels is empty. But it seems things will be pretty broken if that's the case. random.nextInt(0) throws, for example. Are we doing this because we aren't confident the state of things when it is called?
There was a problem hiding this comment.
To not relying on the assumption that it is protected by syncContext is one reason.
The other is kinda best practice to always call hasNext() before next. I guess it does not hurt unless that it might hide some unexpected bugs.
|
Want to have a test that verifies (unsure it exists, but want to verify it exists or add it) sticky-TF and requestConnection() cooperate:
|
| if (!connectionAttemptIterator.hasNext()) { | ||
| connectionAttemptIterator = subchannels.values().iterator(); | ||
| } | ||
| if (connectionAttemptIterator.hasNext()) { |
There was a problem hiding this comment.
Is there a reason to have this condition at all? The only way it would produce results is if subchannels is empty. But it seems things will be pretty broken if that's the case. random.nextInt(0) throws, for example. Are we doing this because we aren't confident the state of things when it is called?
ejona86
added
the
TODO:backport
…es (#9084) (#9094) per gRFC change grpc/grpc#29332: Apply new aggregation rule: If there is at least one subchannel in state TRANSIENT_FAILURE and there are more than one subchannel, report state CONNECTING. If we hit this rule, proactively start a subchannel connection attempt.
YifeiZhuang
removed
the
TODO:backport
…es (#9084) (#9107) part of ring-hash part of the change grpc/grpc#29332: Apply new aggregation rule: If there is at least one subchannel in state TRANSIENT_FAILURE and there are more than one subchannel, report state CONNECTING. If we hit this rule, proactively start a subchannel connection attempt.
…es (#9084) (#9108) part of ring-hash part of the change grpc/grpc#29332: Apply new aggregation rule: If there is at least one subchannel in state TRANSIENT_FAILURE and there are more than one subchannel, report state CONNECTING. If we hit this rule, proactively start a subchannel connection attempt.
part of ring-hash part of the change grpc/grpc#29332:
Apply new aggregation rule: If there is at least one subchannel in state TRANSIENT_FAILURE and there are more than one subchannel, report state CONNECTING. If we hit this rule, proactively start a subchannel connection attempt.