Skip to content

Commit 38952b0

Browse files
authored
grpclb: cancel stream instead of halfClose when balancer shutdown
Previously, if the remote balancer doesn't close the stream after receiving the half-close, the OOB channel will hang there forever. It's safer to always cancel on the client-side so that the OOB channel can be cleaned up timely, regardless of what the remote balancer does.
1 parent b009e92 commit 38952b0

File tree

2 files changed

+8
-8
lines changed

2 files changed

+8
-8
lines changed

grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ private void shutdownLbComm() {
279279

280280
private void shutdownLbRpc() {
281281
if (lbStream != null) {
282-
lbStream.close(null);
282+
lbStream.close(Status.CANCELLED.withDescription("balancer shutdown").asException());
283283
// lbStream will be set to null in LbStream.cleanup()
284284
}
285285
}
@@ -668,17 +668,13 @@ private void handleStreamClosed(Status error) {
668668
helper.refreshNameResolution();
669669
}
670670

671-
void close(@Nullable Exception error) {
671+
void close(Exception error) {
672672
if (closed) {
673673
return;
674674
}
675675
closed = true;
676676
cleanUp();
677-
if (error == null) {
678-
lbRequestWriter.onCompleted();
679-
} else {
680-
lbRequestWriter.onError(error);
681-
}
677+
lbRequestWriter.onError(error);
682678
}
683679

684680
private void cleanUp() {

grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import io.grpc.ManagedChannel;
6666
import io.grpc.Metadata;
6767
import io.grpc.Status;
68+
import io.grpc.Status.Code;
6869
import io.grpc.SynchronizationContext;
6970
import io.grpc.grpclb.GrpclbState.BackendEntry;
7071
import io.grpc.grpclb.GrpclbState.DropEntry;
@@ -1835,7 +1836,10 @@ private void subtestShutdownWithoutSubchannel(String childPolicy) throws Excepti
18351836

18361837
verify(requestObserver, never()).onCompleted();
18371838
balancer.shutdown();
1838-
verify(requestObserver).onCompleted();
1839+
ArgumentCaptor<Throwable> throwableCaptor = ArgumentCaptor.forClass(Throwable.class);
1840+
verify(requestObserver).onError(throwableCaptor.capture());
1841+
assertThat(Status.fromThrowable(throwableCaptor.getValue()).getCode())
1842+
.isEqualTo(Code.CANCELLED);
18391843
}
18401844

18411845
@SuppressWarnings("deprecation")

0 commit comments

Comments
 (0)