Skip to content

Commit 47f960a

Browse files
committed
xds: Fix intermittent failure in RingHashLoadBalancerTest.allSubchannelsInTransientFailure
Deliver TRANSIENT_FAILURE in deterministic server order using subchannelList instead of iterating over subchannels.values() (HashMap), and expect 2 connections. Why this became flaky with the new PickFirst LB: Old PickFirst unconditionally invoked subchannel.requestConnection() on each call without checking connection state, so triggering proactive connections twice always produced 2 connection requests regardless of iteration order (either [A, B] or [A, A]). New PickFirst deduplicates connection requests if a subchannel is already in CONNECTING state. When iterating over a HashMap, if the same subchannel was triggered twice, New PickFirst deduplicated it to 1 connection, leading to the assumption in grpc#13094 that New PickFirst would always yield 1 connection. However, if two distinct subchannels were triggered (e.g. server1 then server2), New PickFirst does not deduplicate and yields 2 connections, causing verifyConnection(1) to fail intermittently. Fix this by iterating deterministically in server order, which reliably triggers two distinct subchannels (server1 and server2) in both old and new PickFirst. TAG=agy CONV=d59ec8a0-3bb7-41f8-bb11-76e843a2dada
1 parent 2c9b067 commit 47f960a

1 file changed

Lines changed: 3 additions & 3 deletions

File tree

‎xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -719,17 +719,17 @@ public void allSubchannelsInTransientFailure() {
719719
// Map each server address to exactly one ring entry.
720720
RingHashConfig config = new RingHashConfig(3, 3, "");
721721
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
722-
initializeLbSubchannels(config, servers);
722+
List<Subchannel> subchannelList = initializeLbSubchannels(config, servers);
723723

724724
// Bring all subchannels to TRANSIENT_FAILURE.
725-
for (Subchannel subchannel : subchannels.values()) {
725+
for (Subchannel subchannel : subchannelList) {
726726
deliverSubchannelState(subchannel, ConnectivityStateInfo.forTransientFailure(
727727
Status.UNAVAILABLE.withDescription(
728728
subchannel.getAddresses().getAddresses() + " unreachable")));
729729
}
730730
verify(helper, atLeastOnce())
731731
.updateBalancingState(eq(TRANSIENT_FAILURE), pickerCaptor.capture());
732-
verifyConnection(PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2);
732+
verifyConnection(2);
733733

734734
// Picking subchannel triggers connection. RPC hash hits server0.
735735
PickSubchannelArgs args = getDefaultPickSubchannelArgsForServer(0);

0 commit comments

Comments
 (0)