Skip to content

Commit fc43144

Browse files
authored
core: Reset configSelector on realChannel while entering IDLE state (grpc#12832)
ManagedChannel will be stuck in IDLE state when xDS control plane doesn't have a resource anymore. The scenario is following: 1. Channel is open for a xds resource. 2. XdsNameResolver subscribes to the resource on xDS control plane. 3. The resource is removed from xDS control plane for extended period of time (unhealthy for more than the idle timeout on Channel) 4. Channel enters into TRANSIENT_FAILURE 5. The Idle timeout triggers and Channel shutdowns XdsNameResolver and other resources. xDS watchers are removed. 6. The resource comes back online on xDS control plane. 7. A new GRPC call is executed targeting the channel. 8. The channel stays in IDLE state and reports: "io.grpc.StatusRuntimeException: UNAVAILABLE: LDS resource xxxx does not exist nodeID: yyyy" because realChannel.configSelector still points to old state. ``` public <ReqT, RespT> ClientCall<ReqT, RespT> newCall( MethodDescriptor<ReqT, RespT> method, CallOptions callOptions) { if (configSelector.get() != INITIAL_PENDING_SELECTOR) { return newClientCall(method, callOptions); } ... ```
1 parent 56195da commit fc43144

2 files changed

Lines changed: 118 additions & 6 deletions

File tree

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ private void enterIdleMode() {
433433
// which are bugs.
434434
shutdownNameResolverAndLoadBalancer(true);
435435
delayedTransport.reprocess(null);
436+
realChannel.updateConfigSelector(INITIAL_PENDING_SELECTOR);
436437
channelLogger.log(ChannelLogLevel.INFO, "Entering IDLE state");
437438
channelStateManager.gotoState(IDLE);
438439
// If the inUseStateAggregator still considers pending calls to be queued up or the delayed
@@ -934,7 +935,8 @@ public void run() {
934935
void updateConfigSelector(@Nullable InternalConfigSelector config) {
935936
InternalConfigSelector prevConfig = configSelector.get();
936937
configSelector.set(config);
937-
if (prevConfig == INITIAL_PENDING_SELECTOR && pendingCalls != null) {
938+
if (prevConfig == INITIAL_PENDING_SELECTOR
939+
&& config != INITIAL_PENDING_SELECTOR && pendingCalls != null) {
938940
for (RealChannel.PendingCall<?, ?> pendingCall : pendingCalls) {
939941
pendingCall.reprocess();
940942
}

core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java

Lines changed: 115 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@
4242
import io.grpc.ChannelLogger;
4343
import io.grpc.ClientCall;
4444
import io.grpc.ClientInterceptor;
45+
import io.grpc.ClientStreamTracer;
4546
import io.grpc.ConnectivityState;
4647
import io.grpc.EquivalentAddressGroup;
4748
import io.grpc.IntegerMarshaller;
49+
import io.grpc.InternalConfigSelector;
4850
import io.grpc.LoadBalancer;
4951
import io.grpc.LoadBalancer.CreateSubchannelArgs;
5052
import io.grpc.LoadBalancer.Helper;
@@ -61,6 +63,7 @@
6163
import io.grpc.MethodDescriptor;
6264
import io.grpc.MethodDescriptor.MethodType;
6365
import io.grpc.NameResolver;
66+
import io.grpc.NameResolver.ConfigOrError;
6467
import io.grpc.NameResolver.ResolutionResult;
6568
import io.grpc.NameResolverProvider;
6669
import io.grpc.Status;
@@ -79,6 +82,7 @@
7982
import java.util.concurrent.BlockingQueue;
8083
import java.util.concurrent.Executor;
8184
import java.util.concurrent.TimeUnit;
85+
import java.util.concurrent.atomic.AtomicInteger;
8286
import java.util.concurrent.atomic.AtomicReference;
8387
import org.junit.After;
8488
import org.junit.Before;
@@ -314,6 +318,66 @@ public void pendingCallExitsIdleAfterEnter() throws Exception {
314318
verify(mockNameResolver, times(2)).start(any(NameResolver.Listener2.class));
315319
}
316320

321+
@Test
322+
public void newCallAfterIdleWaitsForFreshConfigSelector() {
323+
AtomicInteger staleSelectCount = new AtomicInteger();
324+
AtomicInteger freshSelectCount = new AtomicInteger();
325+
Attributes staleSelectorAttrs = Attributes.newBuilder()
326+
.set(InternalConfigSelector.KEY, countingConfigSelector(staleSelectCount))
327+
.build();
328+
Attributes freshSelectorAttrs = Attributes.newBuilder()
329+
.set(InternalConfigSelector.KEY, countingConfigSelector(freshSelectCount))
330+
.build();
331+
ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
332+
333+
assertEquals(ConnectivityState.IDLE, channel.getState(true));
334+
deliverResolutionResult(staleSelectorAttrs, 1);
335+
336+
channel.enterIdle();
337+
338+
Metadata headers = new Metadata();
339+
ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
340+
call.start(mockCallListener, headers);
341+
342+
assertEquals(0, staleSelectCount.get());
343+
assertEquals(0, freshSelectCount.get());
344+
verify(mockLoadBalancerProvider, times(2)).newLoadBalancer(helperCaptor.capture());
345+
Helper helper = helperCaptor.getAllValues().get(1);
346+
347+
deliverResolutionResult(freshSelectorAttrs, 2);
348+
349+
assertEquals(0, staleSelectCount.get());
350+
verifyPendingCallDrainedToTransport(helper);
351+
assertEquals(1, freshSelectCount.get());
352+
353+
channel.enterIdle();
354+
}
355+
356+
@Test
357+
public void enterIdleWithInitialPendingCallDoesNotReprocessUntilConfigArrives() {
358+
ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
359+
Metadata headers = new Metadata();
360+
361+
assertEquals(ConnectivityState.IDLE, channel.getState(true));
362+
ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
363+
call.start(mockCallListener, headers);
364+
365+
channel.enterIdle();
366+
367+
assertFalse(channel.isInPanicMode());
368+
verify(mockCallListener, never()).onClose(any(Status.class), any(Metadata.class));
369+
verify(mockLoadBalancerProvider, times(2)).newLoadBalancer(helperCaptor.capture());
370+
Helper helper = helperCaptor.getAllValues().get(1);
371+
372+
deliverResolutionResult(Attributes.EMPTY, 2);
373+
374+
assertFalse(channel.isInPanicMode());
375+
verify(mockCallListener, never()).onClose(any(Status.class), any(Metadata.class));
376+
verifyPendingCallDrainedToTransport(helper);
377+
378+
channel.enterIdle();
379+
}
380+
317381
@Test
318382
public void delayedTransportExitsIdleAfterEnter() throws Exception {
319383
// Start a new call that will go to the delayed transport
@@ -612,18 +676,64 @@ public void run() {
612676
}
613677

614678
private void deliverResolutionResult() {
615-
verify(mockNameResolver).start(nameResolverListenerCaptor.capture());
679+
deliverResolutionResult(Attributes.EMPTY, 1);
680+
}
681+
682+
private void deliverResolutionResult(Attributes attributes, int nameResolverStartCount) {
683+
verify(mockNameResolver, times(nameResolverStartCount)).start(
684+
nameResolverListenerCaptor.capture());
616685
// Simulate new address resolved to make sure the LoadBalancer is correctly linked to
617686
// the NameResolver.
618-
ResolutionResult resolutionResult =
687+
ResolutionResult.Builder resultBuilder =
619688
ResolutionResult.newBuilder()
620689
.setAddressesOrError(StatusOr.fromValue(servers))
621-
.setAttributes(Attributes.EMPTY)
622-
.build();
623-
nameResolverListenerCaptor.getValue().onResult(resolutionResult);
690+
.setAttributes(attributes);
691+
if (attributes.get(InternalConfigSelector.KEY) != null) {
692+
resultBuilder.setServiceConfig(
693+
ConfigOrError.fromConfig(ManagedChannelServiceConfig.empty()));
694+
}
695+
ResolutionResult resolutionResult = resultBuilder.build();
696+
List<NameResolver.Listener2> listeners = nameResolverListenerCaptor.getAllValues();
697+
listeners.get(listeners.size() - 1).onResult(resolutionResult);
624698
executor.runDueTasks();
625699
}
626700

701+
private void verifyPendingCallDrainedToTransport(Helper helper) {
702+
ClientStream mockStream = mock(ClientStream.class);
703+
Subchannel subchannel = createSubchannelSafely(helper, servers.get(0), Attributes.EMPTY);
704+
requestConnectionSafely(helper, subchannel);
705+
MockClientTransportInfo transportInfo = newTransports.poll();
706+
when(transportInfo.transport.newStream(
707+
same(method), any(Metadata.class), any(CallOptions.class),
708+
any(ClientStreamTracer[].class)))
709+
.thenReturn(mockStream);
710+
transportInfo.listener.transportReady();
711+
SubchannelPicker picker = mock(SubchannelPicker.class);
712+
when(picker.pickSubchannel(any(PickSubchannelArgs.class)))
713+
.thenReturn(PickResult.withSubchannel(subchannel));
714+
715+
updateBalancingStateSafely(helper, READY, picker);
716+
executor.runDueTasks();
717+
718+
verify(transportInfo.transport).newStream(
719+
same(method), any(Metadata.class), any(CallOptions.class),
720+
any(ClientStreamTracer[].class));
721+
verify(mockStream).start(any(ClientStreamListener.class));
722+
}
723+
724+
private static InternalConfigSelector countingConfigSelector(
725+
final AtomicInteger selectCount) {
726+
return new InternalConfigSelector() {
727+
@Override
728+
public Result selectConfig(PickSubchannelArgs args) {
729+
selectCount.incrementAndGet();
730+
return Result.newBuilder()
731+
.setConfig(ManagedChannelServiceConfig.empty())
732+
.build();
733+
}
734+
};
735+
}
736+
627737
private static void requestConnectionSafely(Helper helper, final Subchannel subchannel) {
628738
helper.getSynchronizationContext().execute(
629739
new Runnable() {

0 commit comments

Comments
 (0)