Skip to content

Commit 2448c8b

Browse files
authored
util: Replace BUFFER_PICKER with FixedResultPicker
I think at some point there were more usages in the tests. But now it is pretty easy. PriorityLb.ChildLbState.picker is initialized to FixedResultPicker(NoResult). So now that GracefulSwitchLb is using the same picker, equals() is able to de-dup an update.
1 parent 2e260a4 commit 2448c8b

File tree

3 files changed

+10
-22
lines changed

3 files changed

+10
-22
lines changed

util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static com.google.common.base.Preconditions.checkNotNull;
2020
import static com.google.common.base.Preconditions.checkState;
2121

22-
import com.google.common.annotations.VisibleForTesting;
2322
import com.google.common.base.MoreObjects;
2423
import com.google.common.base.Objects;
2524
import io.grpc.ConnectivityState;
@@ -66,19 +65,6 @@ public void handleNameResolutionError(final Status error) {
6665
public void shutdown() {}
6766
};
6867

69-
@VisibleForTesting
70-
static final SubchannelPicker BUFFER_PICKER = new SubchannelPicker() {
71-
@Override
72-
public PickResult pickSubchannel(PickSubchannelArgs args) {
73-
return PickResult.withNoResult();
74-
}
75-
76-
@Override
77-
public String toString() {
78-
return "BUFFER_PICKER";
79-
}
80-
};
81-
8268
private final Helper helper;
8369

8470
// While the new policy is not fully switched on, the pendingLb is handling new updates from name
@@ -128,7 +114,7 @@ private void switchToInternal(LoadBalancer.Factory newBalancerFactory) {
128114
pendingLb = defaultBalancer;
129115
pendingBalancerFactory = null;
130116
pendingState = ConnectivityState.CONNECTING;
131-
pendingPicker = BUFFER_PICKER;
117+
pendingPicker = new FixedResultPicker(PickResult.withNoResult());
132118

133119
if (newBalancerFactory.equals(currentBalancerFactory)) {
134120
return;

util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import static io.grpc.ConnectivityState.IDLE;
2222
import static io.grpc.ConnectivityState.READY;
2323
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
24-
import static io.grpc.util.GracefulSwitchLoadBalancer.BUFFER_PICKER;
2524
import static org.mockito.ArgumentMatchers.any;
2625
import static org.mockito.ArgumentMatchers.eq;
2726
import static org.mockito.Mockito.inOrder;
@@ -521,7 +520,11 @@ public void switchWhileOldPolicyGoesFromReadyToNotReadyWhileNewPolicyStillIdle()
521520
helper0.updateBalancingState(CONNECTING, picker);
522521

523522
verify(mockHelper, never()).updateBalancingState(CONNECTING, picker);
524-
inOrder.verify(mockHelper).updateBalancingState(CONNECTING, BUFFER_PICKER);
523+
ArgumentCaptor<SubchannelPicker> pickerCaptor = ArgumentCaptor.forClass(SubchannelPicker.class);
524+
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
525+
assertThat(pickerCaptor.getValue().pickSubchannel(mock(PickSubchannelArgs.class)).hasResult())
526+
.isFalse();
527+
525528
inOrder.verify(lb0).shutdown(); // shutdown after update
526529

527530
picker = mock(SubchannelPicker.class);

xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -522,8 +522,7 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() {
522522
.setLoadBalancingPolicyConfig(priorityLbConfig)
523523
.build());
524524
// Nothing important about this verify, other than to provide a baseline
525-
verify(helper, times(2))
526-
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
525+
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
527526
assertThat(fooBalancers).hasSize(1);
528527
assertThat(fooHelpers).hasSize(1);
529528
Helper helper0 = Iterables.getOnlyElement(fooHelpers);
@@ -539,7 +538,7 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() {
539538
helper0.updateBalancingState(
540539
CONNECTING,
541540
EMPTY_PICKER);
542-
verify(helper, times(3))
541+
verify(helper, times(2))
543542
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
544543

545544
// failover happens
@@ -805,7 +804,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() {
805804
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
806805
.setLoadBalancingPolicyConfig(priorityLbConfig)
807806
.build());
808-
verify(helper, times(2)).updateBalancingState(eq(CONNECTING), isA(SubchannelPicker.class));
807+
verify(helper).updateBalancingState(eq(CONNECTING), isA(SubchannelPicker.class));
809808

810809
// LB shutdown and subchannel state change can happen simultaneously. If shutdown runs first,
811810
// any further balancing state update should be ignored.
@@ -843,7 +842,7 @@ public void noDuplicateOverallBalancingStateUpdate() {
843842
.setLoadBalancingPolicyConfig(priorityLbConfig)
844843
.build());
845844

846-
verify(helper, times(6)).updateBalancingState(any(), any());
845+
verify(helper, times(4)).updateBalancingState(any(), any());
847846
}
848847

849848
private void assertLatestConnectivityState(ConnectivityState expectedState) {

0 commit comments

Comments
 (0)