Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core: keep round_robin lb subchannel in TRANSIENT_FAILURE until becoming READY #6657

Merged
merged 9 commits into from Apr 6, 2020
Merged

core: keep round_robin lb subchannel in TRANSIENT_FAILURE until becoming READY #6657

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
246c81b
Make round_robin subchannel stay in TRANSIENT_FAILURE until becoming …
voidzcy Jan 29, 2020
60f8f9d
Merge branch 'master' of github.com:grpc/grpc-java into bugfix/round_…
voidzcy Apr 2, 2020
4cf7210
Fix wrong git merge
voidzcy Apr 3, 2020
b3fd8a7
Only need to do state transition based on RR LB's state without looki…
voidzcy Apr 3, 2020
654dd7b
May still want to update picker even if state has not change to ready.
voidzcy Apr 3, 2020
1108da5
Manage each subchannel's state, as they operates individually.
voidzcy Apr 3, 2020
72e60ab
Improve test to reflect each subchannel operates individually.
voidzcy Apr 3, 2020
348614d
Fix existing test for verifying subchannel state after TF.
voidzcy Apr 3, 2020
5c91cbd
Make verification for subchannel state more explicit.
voidzcy Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java
View file
Open in desktop
Expand Up @@ -142,7 +142,13 @@ private void processSubchannelState(Subchannel subchannel, ConnectivityStateInfo
if (stateInfo.getState() == IDLE) {
subchannel.requestConnection();
}
getSubchannelStateInfoRef(subchannel).value = stateInfo;
Ref<ConnectivityStateInfo> subchannelStateRef = getSubchannelStateInfoRef(subchannel);
if (subchannelStateRef.value.getState().equals(TRANSIENT_FAILURE)) {
if (stateInfo.getState().equals(CONNECTING) || stateInfo.getState().equals(IDLE)) {
return;
}
}
subchannelStateRef.value = stateInfo;
updateBalancingState();
}

Expand Down
41 changes: 37 additions & 4 deletions core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java
View file
Open in desktop
Expand Up @@ -279,21 +279,54 @@ public void pickAfterStateChange() throws Exception {
Status error = Status.UNKNOWN.withDescription("¯\\_(ツ)_//¯");
deliverSubchannelState(subchannel,
ConnectivityStateInfo.forTransientFailure(error));
assertThat(subchannelStateInfo.value).isEqualTo(
ConnectivityStateInfo.forTransientFailure(error));
assertThat(subchannelStateInfo.value.getState()).isEqualTo(TRANSIENT_FAILURE);
assertThat(subchannelStateInfo.value.getStatus()).isEqualTo(error);
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
assertThat(pickerCaptor.getValue()).isInstanceOf(EmptyPicker.class);

deliverSubchannelState(subchannel,
ConnectivityStateInfo.forNonError(IDLE));
assertThat(subchannelStateInfo.value).isEqualTo(
ConnectivityStateInfo.forNonError(IDLE));
assertThat(subchannelStateInfo.value.getState()).isEqualTo(TRANSIENT_FAILURE);
assertThat(subchannelStateInfo.value.getStatus()).isEqualTo(error);

verify(subchannel, times(2)).requestConnection();
verify(mockHelper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
verifyNoMoreInteractions(mockHelper);
}

@Test
public void stayTransientFailureUntilReady() {
InOrder inOrder = inOrder(mockHelper);
loadBalancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY)
.build());

inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class));

// Simulate state transitions for each subchannel individually.
for (Subchannel sc : loadBalancer.getSubchannels()) {
deliverSubchannelState(
sc,
ConnectivityStateInfo
.forTransientFailure(Status.UNKNOWN.withDescription("connection broken")));
deliverSubchannelState(
sc,
ConnectivityStateInfo.forNonError(IDLE));
deliverSubchannelState(
sc,
ConnectivityStateInfo.forNonError(CONNECTING));
}
inOrder.verify(mockHelper).updateBalancingState(eq(TRANSIENT_FAILURE), isA(EmptyPicker.class));
inOrder.verifyNoMoreInteractions();

Subchannel subchannel = loadBalancer.getSubchannels().iterator().next();
deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY));
inOrder.verify(mockHelper).updateBalancingState(eq(READY), isA(ReadyPicker.class));

verify(mockHelper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
verifyNoMoreInteractions(mockHelper);
}

@Test
public void pickerRoundRobin() throws Exception {
Subchannel subchannel = mock(Subchannel.class);
Expand Down