A42 update: fix ring_hash connectivity state aggregation rules

[markdroth]

No description provided.

[ejona86]

CC @dfawley

[apolcyn]

Just to confirm, consider the following case:

• ring hash subchannels are all unreachable in an unresponsive way (so it takes e.g. 30 seconds to notice each connection failure)
• ring hash first under a priority policy, and a working failover priority follows it
• channel and the LB policy tree are just starting up, i.e. first RPC is made on a new channel

Does this mean we can now rely on the priority failover timer to fail over after 10 seconds? (where before we needed to wait for ring hash to enter TF)

[markdroth]

Yes, it does mean that we'll failover faster in the case where all backends are blackholed.

The down-side of this is that we may wind up failing over without ever having tried more than one address. So while this will fail over more quickly in the case where all of the addresses are blackholed, if there is a case where only the first address we try is blackholed but all of the other addresses are working fine, we will wind up failing over when we ideally shouldn't.

If that becomes a problem, and if can ever agree on the semantics for happy eyeballs, we could potentially use something like that to improve this in the future.

[apolcyn]

Is this accurate, that there can be addresses which ring hash will never attempt?

I might be wrong, but I thought that if every connection attempt failed, that ring hash will eventually circle around and try every address?

I'm asking this because, if ring hash will eventually attempt every address, then considering the new priority changes to properly handle the fail over timeout with ring hash, I'm wondering if this heuristic to enter TRANSIENT_FAILURE after encountering two connection failures is still worth having - i.e. what problem would it solve that isn't solved by priority fail over.

[markdroth]

Yes, you're right, it will eventually try all of the subchannels. However, this change is still needed for two reasons:

1. The priority policy uses an idempotent algorithm for choosing a priority that is triggered every time a child's state changes, and that algorithm treats IDLE the same as READY -- i.e., the state of the failover timer matters only when the child is reporting CONNECTING, so if ring_hash continues to report IDLE after the failover timer fires, we'll wind up re-selecting it as soon as the newly created next priority reports CONNECTING, which is not what we want. (To say this another way, the priority policy assumes that a policy in IDLE will transition to CONNECTING as soon as it gets a pick.)
2. The ring_hash policy is designed to not be xDS-specific; eventually, we'd like to be able to use it as a top-level policy even without xDS. At that point, it would still be necessary to have the same hueristic here so that the channel will properly go into TRANSIENT_FAILURE when things are not working, rather than staying in IDLE indefinitely. If things are not working, we want non-wait_for_ready RPCs to fail quickly.

I've updated the wording here to try to clarify this a bit.

[apolcyn]

Copying over context from chat FTR:

When ring_hash is under priority, it could stay in connecting for however long it took until reaching TD, and priority would do the right thing.

However, when ring_hash is a top-level policy, we need to balance the goals of failing fast in some cases and using all failover addresses in others.

My only thought about waiting until every subchannel enters TF is that it may be easier for users to understand - that time taken for ring hash to reach TF is proportional to ring hash's address list size (i.e. we remove the need to understand this heuristic). But OTOH, taking forever to reach TF introduces its own problems.

[markdroth]

Yeah, we really can't wait until every single subchannel has been attempted before going TF, since that would basically turn every RPC into a wait_for_ready RPC, which is definitely not what we want.

This is a heuristic, and like any heuristic, it's an imperfect attempt to balance between competing objectives. No matter what value we choose, someone can argue that a different value would be better in their case, but we can't choose the value they want without making it worse in some other case.

At the end of the day, I think two is the right number here. We definitely don't want to do just one, because having one individual backend unreachable is probably not uncommon, and we don't want to fail all RPCs in that case. But it's fairly unlikely that the first two backends that we happen to try are both independently down at the same time without there being some broader reachability issue that will also affect any other backend we might try. Increasing the number to three might give us slightly more confidence that there is a broader reachability issue that will also affect any other backend, but that slight boost in confidence doesn't seem like enough to justify the additional delay in failing non-wait_for_ready RPCs. And the trade-off gets dramatically worse as you go up from three: the increased confidence becomes smaller and smaller, and the delay before failing non-wait_for_ready RPCs becomes larger and larger. So I think two is the right sweet-spot here.