From 47f960a3783e42044e947fc104cf71d24a2b421a Mon Sep 17 00:00:00 2001 From: Kannan J Date: Mon, 19 Oct 2026 07:20:55 +0000 Subject: [PATCH] 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 #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 --- xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java index 9c7f8dc62e0..00d5e8996ff 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java @@ -719,17 +719,17 @@ public void allSubchannelsInTransientFailure() { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3, ""); List servers = createWeightedServerAddrs(1, 1, 1); - initializeLbSubchannels(config, servers); + List subchannelList = initializeLbSubchannels(config, servers); // Bring all subchannels to TRANSIENT_FAILURE. - for (Subchannel subchannel : subchannels.values()) { + for (Subchannel subchannel : subchannelList) { deliverSubchannelState(subchannel, ConnectivityStateInfo.forTransientFailure( Status.UNAVAILABLE.withDescription( subchannel.getAddresses().getAddresses() + " unreachable"))); } verify(helper, atLeastOnce()) .updateBalancingState(eq(TRANSIENT_FAILURE), pickerCaptor.capture()); - verifyConnection(PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2); + verifyConnection(2); // Picking subchannel triggers connection. RPC hash hits server0. PickSubchannelArgs args = getDefaultPickSubchannelArgsForServer(0);