grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

core: Transition to CONNECTING immediately when exiting idle 

The name resolver takes some time before it returns addresses. While waiting the channel will be IDLE instead of the proper CONNECTING. This generally doesn't matter since RPCs behave similarly for IDLE and CONNECTING, but is confusing for users when watching channel.getState() closely.

Fixes #10517.
This commit is contained in:
Kannan J 2024-03-28 00:10:13 +05:30 committed by GitHub
parent f7ee5f3182
commit 2c5f0c22cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 24 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
View File

@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static io.grpc.ClientStreamTracer.NAME_RESOLUTION_DELAYED; import static io.grpc.ClientStreamTracer.NAME_RESOLUTION_DELAYED;
import static io.grpc.ConnectivityState.CONNECTING;
import static io.grpc.ConnectivityState.IDLE; import static io.grpc.ConnectivityState.IDLE;
import static io.grpc.ConnectivityState.SHUTDOWN; import static io.grpc.ConnectivityState.SHUTDOWN;
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE; import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
@ -423,6 +424,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
// may throw. We don't want to confuse our state, even if we will enter panic mode. // may throw. We don't want to confuse our state, even if we will enter panic mode.
this.lbHelper = lbHelper; this.lbHelper = lbHelper;
channelStateManager.gotoState(CONNECTING);
NameResolverListener listener = new NameResolverListener(lbHelper, nameResolver); NameResolverListener listener = new NameResolverListener(lbHelper, nameResolver);
nameResolver.start(listener); nameResolver.start(listener);
nameResolverStarted = true; nameResolverStarted = true;

45
core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
View File

@ -2338,7 +2338,7 @@ public class ManagedChannelImplTest {
channelBuilder.nameResolverFactory( channelBuilder.nameResolverFactory(
new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build()); new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build());
createChannel(); createChannel();
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
updateBalancingStateSafely(helper, TRANSIENT_FAILURE, mockPicker); updateBalancingStateSafely(helper, TRANSIENT_FAILURE, mockPicker);
assertEquals(TRANSIENT_FAILURE, channel.getState(false)); assertEquals(TRANSIENT_FAILURE, channel.getState(false));
@ -2395,21 +2395,21 @@ public class ManagedChannelImplTest {
channelBuilder.nameResolverFactory( channelBuilder.nameResolverFactory(
new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build()); new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build());
createChannel(); createChannel();
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
channel.notifyWhenStateChanged(IDLE, onStateChanged); channel.notifyWhenStateChanged(CONNECTING, onStateChanged);
executor.runDueTasks(); executor.runDueTasks();
assertFalse(stateChanged.get()); assertFalse(stateChanged.get());
// state change from IDLE to CONNECTING // state change from CONNECTING to IDLE
updateBalancingStateSafely(helper, CONNECTING, mockPicker); updateBalancingStateSafely(helper, IDLE, mockPicker);
// onStateChanged callback should run // onStateChanged callback should run
executor.runDueTasks(); executor.runDueTasks();
assertTrue(stateChanged.get()); assertTrue(stateChanged.get());
// clear and test form CONNECTING // clear and test form IDLE
stateChanged.set(false); stateChanged.set(false);
channel.notifyWhenStateChanged(IDLE, onStateChanged); channel.notifyWhenStateChanged(CONNECTING, onStateChanged);
// onStateChanged callback should run immediately // onStateChanged callback should run immediately
executor.runDueTasks(); executor.runDueTasks();
assertTrue(stateChanged.get()); assertTrue(stateChanged.get());
@ -2428,8 +2428,8 @@ public class ManagedChannelImplTest {
channelBuilder.nameResolverFactory( channelBuilder.nameResolverFactory(
new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build()); new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build());
createChannel(); createChannel();
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
channel.notifyWhenStateChanged(IDLE, onStateChanged); channel.notifyWhenStateChanged(CONNECTING, onStateChanged);
executor.runDueTasks(); executor.runDueTasks();
assertFalse(stateChanged.get()); assertFalse(stateChanged.get());
@ -2452,9 +2452,6 @@ public class ManagedChannelImplTest {
long idleTimeoutMillis = 2000L; long idleTimeoutMillis = 2000L;
channelBuilder.idleTimeout(idleTimeoutMillis, TimeUnit.MILLISECONDS); channelBuilder.idleTimeout(idleTimeoutMillis, TimeUnit.MILLISECONDS);
createChannel(); createChannel();
assertEquals(IDLE, channel.getState(false));
updateBalancingStateSafely(helper, CONNECTING, mockPicker);
assertEquals(CONNECTING, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
timer.forwardNanos(TimeUnit.MILLISECONDS.toNanos(idleTimeoutMillis)); timer.forwardNanos(TimeUnit.MILLISECONDS.toNanos(idleTimeoutMillis));
@ -2677,11 +2674,11 @@ public class ManagedChannelImplTest {
// Updating on the old helper (whose balancer has been shutdown) does not change the channel // Updating on the old helper (whose balancer has been shutdown) does not change the channel
// state. // state.
updateBalancingStateSafely(helper, CONNECTING, mockPicker); updateBalancingStateSafely(helper, IDLE, mockPicker);
assertEquals(IDLE, channel.getState(false));
updateBalancingStateSafely(helper2, CONNECTING, mockPicker);
assertEquals(CONNECTING, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
updateBalancingStateSafely(helper2, IDLE, mockPicker);
assertEquals(IDLE, channel.getState(false));
} }
@Test @Test
@ -2695,7 +2692,7 @@ public class ManagedChannelImplTest {
.setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress))) .setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
.build()); .build());
createChannel(); createChannel();
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
// This call will be buffered in delayedTransport // This call will be buffered in delayedTransport
ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT); ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
@ -2790,7 +2787,7 @@ public class ManagedChannelImplTest {
// enterIdle() will shut down the name resolver and lb policy used to get a pick for the delayed // enterIdle() will shut down the name resolver and lb policy used to get a pick for the delayed
// call // call
channel.enterIdle(); channel.enterIdle();
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
// enterIdle() will restart the delayed call by exiting idle. This creates a new helper. // enterIdle() will restart the delayed call by exiting idle. This creates a new helper.
ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class); ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(Helper.class);
@ -2912,14 +2909,14 @@ public class ManagedChannelImplTest {
channelBuilder.nameResolverFactory( channelBuilder.nameResolverFactory(
new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build()); new FakeNameResolverFactory.Builder(expectedUri).setResolvedAtStart(false).build());
createChannel(); createChannel();
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
Runnable onStateChanged = mock(Runnable.class); Runnable onStateChanged = mock(Runnable.class);
channel.notifyWhenStateChanged(IDLE, onStateChanged); channel.notifyWhenStateChanged(CONNECTING, onStateChanged);
updateBalancingStateSafely(helper, SHUTDOWN, mockPicker); updateBalancingStateSafely(helper, SHUTDOWN, mockPicker);
assertEquals(IDLE, channel.getState(false)); assertEquals(CONNECTING, channel.getState(false));
executor.runDueTasks(); executor.runDueTasks();
verify(onStateChanged, never()).run(); verify(onStateChanged, never()).run();
} }
@ -3222,8 +3219,6 @@ public class ManagedChannelImplTest {
verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture()); verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
helper = helperCaptor.getValue(); helper = helperCaptor.getValue();
assertEquals(IDLE, getStats(channel).state);
updateBalancingStateSafely(helper, CONNECTING, mockPicker);
assertEquals(CONNECTING, getStats(channel).state); assertEquals(CONNECTING, getStats(channel).state);
AbstractSubchannel subchannel = AbstractSubchannel subchannel =
@ -3434,7 +3429,7 @@ public class ManagedChannelImplTest {
assertEquals(READY, getStats(oobChannel).state); assertEquals(READY, getStats(oobChannel).state);
// oobchannel state is separate from the ManagedChannel // oobchannel state is separate from the ManagedChannel
assertEquals(IDLE, getStats(channel).state); assertEquals(CONNECTING, getStats(channel).state);
channel.shutdownNow(); channel.shutdownNow();
assertEquals(SHUTDOWN, getStats(channel).state); assertEquals(SHUTDOWN, getStats(channel).state);
assertEquals(SHUTDOWN, getStats(oobChannel).state); assertEquals(SHUTDOWN, getStats(oobChannel).state);
@ -4536,4 +4531,4 @@ public class ManagedChannelImplTest {
return ManagedChannelServiceConfig return ManagedChannelServiceConfig
.fromServiceConfig(rawServiceConfig, true, 3, 4, policySelection); .fromServiceConfig(rawServiceConfig, true, 3, 4, policySelection);
} }
} }

4
core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java
View File

@ -277,7 +277,7 @@ public class ServiceConfigErrorHandlingTest {
assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("12"); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("12");
verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class));
assertThat(channel.getState(true)).isEqualTo(ConnectivityState.IDLE); assertThat(channel.getState(true)).isEqualTo(ConnectivityState.CONNECTING);
reset(mockLoadBalancer); reset(mockLoadBalancer);
nameResolverFactory.servers.clear(); nameResolverFactory.servers.clear();
@ -480,7 +480,7 @@ public class ServiceConfigErrorHandlingTest {
assertThat(newResolvedAddress.getLoadBalancingPolicyConfig()).isEqualTo("1st raw config"); assertThat(newResolvedAddress.getLoadBalancingPolicyConfig()).isEqualTo("1st raw config");
assertThat(channel.getConfigSelector()).isSameInstanceAs(configSelector); assertThat(channel.getConfigSelector()).isSameInstanceAs(configSelector);
verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class));
assertThat(channel.getState(false)).isEqualTo(ConnectivityState.IDLE); assertThat(channel.getState(false)).isEqualTo(ConnectivityState.CONNECTING);
} }
@Test @Test