#20551

Committed 03 Nov 2026 03:24AM UTC coverage: 89.449% (+0.007%) from 89.442%

Build # #20551

Build Type

push

github

Committed by

web-flow

Commit Message

xds: Fix intermittent failure in RingHashLoadBalancerTest.allSubchannelsInTransientFailure (#13130)

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.

Failure test run in fusion2/d9446b33-dc0e-4dee-b901-ea9b3b57f3c6

Coverage Stats

39413 of 44062 relevant lines covered (89.45%)

0.89 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

87.88

/../netty/src/main/java/io/grpc/netty/ClientTransportLifecycleManager.java

/*
 * Copyright 2016 The gRPC Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package io.grpc.netty;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import io.grpc.Attributes;
import io.grpc.Status;
import io.grpc.internal.DisconnectError;
import io.grpc.internal.ManagedClientTransport;

/** Maintainer of transport lifecycle status. */
final class ClientTransportLifecycleManager {
  private final ManagedClientTransport.Listener listener;
  private boolean transportReady;
  private boolean transportShutdown;
  private boolean transportInUse;
  /** null iff !transportShutdown. */
  private Status shutdownStatus;
  /** null iff !transportShutdown. */
  private boolean transportTerminated;

  public ClientTransportLifecycleManager(ManagedClientTransport.Listener listener) {
    this.listener = listener;
  }

  public Attributes filterAttributes(Attributes attributes) {
    if (transportReady || transportShutdown) {
      return attributes;
    }
    return listener.filterTransport(attributes);
  }

  public void notifyReady() {
    if (transportReady || transportShutdown) {
      return;
    }
    transportReady = true;
    listener.transportReady();
  }

  /**
   * Marks transport as shutdown, but does not set the error status. This must eventually be
   * followed by a call to notifyShutdown.
   */
  public void notifyGracefulShutdown(Status s, DisconnectError disconnectError) {
    if (transportShutdown) {
      return;
    }
    transportShutdown = true;
    listener.transportShutdown(s, disconnectError);
  }

  /** Returns {@code true} if was the first shutdown. */
  @CanIgnoreReturnValue
  public boolean notifyShutdown(Status s, DisconnectError disconnectError) {
    notifyGracefulShutdown(s, disconnectError);
    if (shutdownStatus != null) {
      return false;
    }
    shutdownStatus = s;
    return true;
  }

  public void notifyInUse(boolean inUse) {
    if (inUse == transportInUse) {
      return;
    }
    transportInUse = inUse;
    listener.transportInUse(inUse);
  }

  public void notifyTerminated(Status s, DisconnectError disconnectError) {
    if (transportTerminated) {
      return;
    }
    transportTerminated = true;
    notifyShutdown(s, disconnectError);
    listener.transportTerminated();
  }

  public Status getShutdownStatus() {
    return shutdownStatus;
  }

}

1	/*
2	* Copyright 2016 The gRPC Authors
3	*
4	* Licensed under the Apache License, Version 2.0 (the "License");
5	* you may not use this file except in compliance with the License.
6	* You may obtain a copy of the License at
7	*
8	* http://www.apache.org/licenses/LICENSE-2.0
9	*
10	* Unless required by applicable law or agreed to in writing, software
11	* distributed under the License is distributed on an "AS IS" BASIS,
12	* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13	* See the License for the specific language governing permissions and
14	* limitations under the License.
15	*/
16
17	package io.grpc.netty;
18
19	import com.google.errorprone.annotations.CanIgnoreReturnValue;
20	import io.grpc.Attributes;
21	import io.grpc.Status;
22	import io.grpc.internal.DisconnectError;
23	import io.grpc.internal.ManagedClientTransport;
24
25	/** Maintainer of transport lifecycle status. */
26	final class ClientTransportLifecycleManager {
27	private final ManagedClientTransport.Listener listener;
28	private boolean transportReady;
29	private boolean transportShutdown;
30	private boolean transportInUse;
31	/** null iff !transportShutdown. */
32	private Status shutdownStatus;
33	/** null iff !transportShutdown. */
34	private boolean transportTerminated;
35
36	public ClientTransportLifecycleManager(ManagedClientTransport.Listener listener) {	1 ✔
37	this.listener = listener;	1 ✔
38	}	1 ✔
39
40	public Attributes filterAttributes(Attributes attributes) {
41	if (transportReady \|\| transportShutdown) {	1 ✔
42	return attributes;	×
43	}
44	return listener.filterTransport(attributes);	1 ✔
45	}
46
47	public void notifyReady() {
48	if (transportReady \|\| transportShutdown) {	1 ✔
49	return;	×
50	}
51	transportReady = true;	1 ✔
52	listener.transportReady();	1 ✔
53	}	1 ✔
54
55	/**
56	* Marks transport as shutdown, but does not set the error status. This must eventually be
57	* followed by a call to notifyShutdown.
58	*/
59	public void notifyGracefulShutdown(Status s, DisconnectError disconnectError) {
60	if (transportShutdown) {	1 ✔
61	return;	1 ✔
62	}
63	transportShutdown = true;	1 ✔
64	listener.transportShutdown(s, disconnectError);	1 ✔
65	}	1 ✔
66
67	/** Returns {@code true} if was the first shutdown. */
68	@CanIgnoreReturnValue
69	public boolean notifyShutdown(Status s, DisconnectError disconnectError) {
70	notifyGracefulShutdown(s, disconnectError);	1 ✔
71	if (shutdownStatus != null) {	1 ✔
72	return false;	1 ✔
73	}
74	shutdownStatus = s;	1 ✔
75	return true;	1 ✔
76	}
77
78	public void notifyInUse(boolean inUse) {
79	if (inUse == transportInUse) {	1 ✔
80	return;	×
81	}
82	transportInUse = inUse;	1 ✔
83	listener.transportInUse(inUse);	1 ✔
84	}	1 ✔
85
86	public void notifyTerminated(Status s, DisconnectError disconnectError) {
87	if (transportTerminated) {	1 ✔
88	return;	×
89	}
90	transportTerminated = true;	1 ✔
91	notifyShutdown(s, disconnectError);	1 ✔
92	listener.transportTerminated();	1 ✔
93	}	1 ✔
94
95	public Status getShutdownStatus() {
96	return shutdownStatus;	1 ✔
97	}
98
99	}

grpc / grpc-java / #20551

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous