Skip to content

Commit 860b5cb

Browse files
ejona86ejona86
committed
api: Deprecate LoadBalancer.EMPTY_PICKER
FixedResultPicker is our preferred approach.
1 parent 3b92333 commit 860b5cb

File tree

10 files changed

+100
-56
lines changed

10 files changed

+100
-56
lines changed

‎api/src/main/java/io/grpc/LoadBalancer.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ public abstract class LoadBalancer {
116116
public static final Attributes.Key<Map<String, ?>> ATTR_HEALTH_CHECKING_CONFIG =
117117
Attributes.Key.create("internal:health-checking-config");
118118

119+
/**
120+
* A picker that always returns an erring pick.
121+
*
122+
* @deprecated Use {@code new FixedResultPicker(PickResult.withNoResult())} instead.
123+
*/
124+
@Deprecated
119125
public static final SubchannelPicker EMPTY_PICKER = new SubchannelPicker() {
120126
@Override
121127
public PickResult pickSubchannel(PickSubchannelArgs args) {

‎api/src/testFixtures/java/io/grpc/LoadBalancerMatchers.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2023 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc;
18+
19+
import static org.mockito.Mockito.argThat;
20+
import static org.mockito.Mockito.mock;
21+
22+
import io.grpc.LoadBalancer.PickResult;
23+
import io.grpc.LoadBalancer.PickSubchannelArgs;
24+
import io.grpc.LoadBalancer.SubchannelPicker;
25+
import org.mockito.ArgumentMatcher;
26+
27+
/**
28+
* Mockito matchers for testing LoadBalancers.
29+
*/
30+
public final class LoadBalancerMatchers {
31+
private LoadBalancerMatchers() {}
32+
33+
public static SubchannelPicker pickerReturns(final PickResult result) {
34+
return pickerReturns(new ArgumentMatcher<PickResult>() {
35+
@Override public boolean matches(PickResult obj) {
36+
return result.equals(obj);
37+
}
38+
39+
@Override public String toString() {
40+
return "[equals " + result + "]";
41+
}
42+
});
43+
}
44+
45+
public static SubchannelPicker pickerReturns(Status.Code code) {
46+
return pickerReturns(new ArgumentMatcher<PickResult>() {
47+
@Override public boolean matches(PickResult obj) {
48+
return obj.getStatus() != null && code.equals(obj.getStatus().getCode());
49+
}
50+
51+
@Override public String toString() {
52+
return "[with code " + code + "]";
53+
}
54+
});
55+
}
56+
57+
public static SubchannelPicker pickerReturns(final ArgumentMatcher<PickResult> matcher) {
58+
return argThat(new ArgumentMatcher<SubchannelPicker>() {
59+
@Override public boolean matches(SubchannelPicker picker) {
60+
return matcher.matches(picker.pickSubchannel(mock(PickSubchannelArgs.class)));
61+
}
62+
63+
@Override public String toString() {
64+
return "[picker returns: result " + matcher + "]";
65+
}
66+
});
67+
}
68+
}

‎util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ protected abstract SubchannelPicker getSubchannelPicker(
7373
Map<Object, SubchannelPicker> childPickers);
7474

7575
protected SubchannelPicker getInitialPicker() {
76-
return EMPTY_PICKER;
76+
return new FixedResultPicker(PickResult.withNoResult());
7777
}
7878

7979
protected SubchannelPicker getErrorPicker(Status error) {

‎xds/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ dependencies {
5959

6060
testImplementation project(':grpc-rls')
6161
testImplementation testFixtures(project(':grpc-core')),
62+
testFixtures(project(':grpc-api')),
6263
testFixtures(project(':grpc-util'))
6364

6465
annotationProcessor libraries.auto.value

‎xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void shutdown() {
172172
private final class ClusterImplLbHelper extends ForwardingLoadBalancerHelper {
173173
private final AtomicLong inFlights;
174174
private ConnectivityState currentState = ConnectivityState.IDLE;
175-
private SubchannelPicker currentPicker = LoadBalancer.EMPTY_PICKER;
175+
private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult());
176176
private List<DropOverload> dropPolicies = Collections.emptyList();
177177
private long maxConcurrentRequests = DEFAULT_PER_CLUSTER_MAX_CONCURRENT_REQUESTS;
178178
@Nullable

‎xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ private void tryNextPriority() {
148148
ChildLbState child =
149149
new ChildLbState(priority, priorityConfigs.get(priority).ignoreReresolution);
150150
children.put(priority, child);
151-
updateOverallState(priority, CONNECTING, LoadBalancer.EMPTY_PICKER);
151+
updateOverallState(priority, CONNECTING, new FixedResultPicker(PickResult.withNoResult()));
152152
// Calling the child's updateResolvedAddresses() can result in tryNextPriority() being
153153
// called recursively. We need to be sure to be done with processing here before it is
154154
// called.
@@ -209,7 +209,7 @@ private final class ChildLbState {
209209
@Nullable ScheduledHandle deletionTimer;
210210
@Nullable String policy;
211211
ConnectivityState connectivityState = CONNECTING;
212-
SubchannelPicker picker = LoadBalancer.EMPTY_PICKER;
212+
SubchannelPicker picker = new FixedResultPicker(PickResult.withNoResult());
213213

214214
ChildLbState(final String priority, boolean ignoreReresolution) {
215215
this.priority = priority;

‎xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ private void updateOverallBalancingState() {
158158
if (overallState == TRANSIENT_FAILURE) {
159159
picker = new WeightedRandomPicker(errorPickers);
160160
} else {
161-
picker = LoadBalancer.EMPTY_PICKER;
161+
picker = new FixedResultPicker(PickResult.withNoResult());
162162
}
163163
} else {
164164
picker = new WeightedRandomPicker(childPickers);
@@ -190,7 +190,7 @@ private static ConnectivityState aggregateState(
190190
private final class ChildHelper extends ForwardingLoadBalancerHelper {
191191
String name;
192192
ConnectivityState currentState = CONNECTING;
193-
SubchannelPicker currentPicker = LoadBalancer.EMPTY_PICKER;
193+
SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult());
194194

195195
private ChildHelper(String name) {
196196
this.name = name;

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
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.LoadBalancer.EMPTY_PICKER;
24+
import static io.grpc.LoadBalancerMatchers.pickerReturns;
2525
import static org.mockito.ArgumentMatchers.any;
2626
import static org.mockito.ArgumentMatchers.eq;
2727
import static org.mockito.ArgumentMatchers.isA;
@@ -76,6 +76,9 @@
7676
/** Tests for {@link PriorityLoadBalancer}. */
7777
@RunWith(JUnit4.class)
7878
public class PriorityLoadBalancerTest {
79+
private static final SubchannelPicker EMPTY_PICKER
80+
= new FixedResultPicker(PickResult.withNoResult());
81+
7982
private final List<LoadBalancer> fooBalancers = new ArrayList<>();
8083
private final List<LoadBalancer> barBalancers = new ArrayList<>();
8184
private final List<Helper> fooHelpers = new ArrayList<>();
@@ -457,6 +460,9 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() {
457460
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
458461
.setLoadBalancingPolicyConfig(priorityLbConfig)
459462
.build());
463+
// Nothing important about this verify, other than to provide a baseline
464+
verify(helper, times(2))
465+
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
460466
assertThat(fooBalancers).hasSize(1);
461467
assertThat(fooHelpers).hasSize(1);
462468
Helper helper0 = Iterables.getOnlyElement(fooHelpers);
@@ -472,7 +478,8 @@ public void connectingResetFailOverIfSeenReadyOrIdleSinceTransientFailure() {
472478
helper0.updateBalancingState(
473479
CONNECTING,
474480
EMPTY_PICKER);
475-
verify(helper, times(2)).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
481+
verify(helper, times(3))
482+
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
476483

477484
// failover happens
478485
fakeClock.forwardTime(10, TimeUnit.SECONDS);

‎xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import static io.grpc.ConnectivityState.CONNECTING;
2121
import static io.grpc.ConnectivityState.READY;
2222
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
23-
import static io.grpc.LoadBalancer.EMPTY_PICKER;
23+
import static io.grpc.LoadBalancerMatchers.pickerReturns;
2424
import static org.mockito.ArgumentMatchers.any;
2525
import static org.mockito.ArgumentMatchers.eq;
2626
import static org.mockito.Mockito.atLeast;
@@ -209,7 +209,7 @@ public void handleResolvedAddresses() {
209209
.setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build())
210210
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
211211
.build());
212-
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
212+
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
213213
assertThat(childBalancers).hasSize(4);
214214
assertThat(childHelpers).hasSize(4);
215215
assertThat(fooLbCreated).isEqualTo(2);
@@ -246,7 +246,8 @@ public void handleResolvedAddresses() {
246246
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
247247
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets))
248248
.build());
249-
verify(helper, atLeast(2)).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
249+
verify(helper, atLeast(2))
250+
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
250251
assertThat(childBalancers).hasSize(5);
251252
assertThat(childHelpers).hasSize(5);
252253
assertThat(fooLbCreated).isEqualTo(3); // One more foo LB created for target4
@@ -288,7 +289,7 @@ public void handleNameResolutionError() {
288289
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
289290
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
290291
.build());
291-
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
292+
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
292293

293294
// Error after child balancers created.
294295
weightedTargetLb.handleNameResolutionError(Status.ABORTED);
@@ -315,7 +316,7 @@ public void balancingStateUpdatedFromChildBalancers() {
315316
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
316317
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
317318
.build());
318-
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
319+
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
319320

320321
// Subchannels to be created for each child balancer.
321322
final SubchannelPicker[] subchannelPickers = new SubchannelPicker[]{
@@ -335,7 +336,8 @@ public void balancingStateUpdatedFromChildBalancers() {
335336
childHelpers.get(1).updateBalancingState(TRANSIENT_FAILURE, failurePickers[1]);
336337
verify(helper, never()).updateBalancingState(
337338
eq(TRANSIENT_FAILURE), any(SubchannelPicker.class));
338-
verify(helper, times(2)).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
339+
verify(helper, times(2))
340+
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
339341

340342
// Another child balancer goes to READY.
341343
childHelpers.get(2).updateBalancingState(READY, subchannelPickers[2]);
@@ -396,7 +398,7 @@ public void raceBetweenShutdownAndChildLbBalancingStateUpdate() {
396398
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
397399
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
398400
.build());
399-
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
401+
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
400402

401403
// LB shutdown and subchannel state change can happen simultaneously. If shutdown runs first,
402404
// any further balancing state update should be ignored.

‎xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@
1717
package io.grpc.xds;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static io.grpc.LoadBalancerMatchers.pickerReturns;
2021
import static io.grpc.xds.XdsLbPolicies.WEIGHTED_TARGET_POLICY_NAME;
2122
import static org.mockito.ArgumentMatchers.eq;
2223
import static org.mockito.ArgumentMatchers.isA;
23-
import static org.mockito.Mockito.argThat;
24-
import static org.mockito.Mockito.mock;
2524
import static org.mockito.Mockito.never;
2625
import static org.mockito.Mockito.verify;
2726
import static org.mockito.Mockito.when;
@@ -34,9 +33,7 @@
3433
import io.grpc.LoadBalancer;
3534
import io.grpc.LoadBalancer.Helper;
3635
import io.grpc.LoadBalancer.PickResult;
37-
import io.grpc.LoadBalancer.PickSubchannelArgs;
3836
import io.grpc.LoadBalancer.ResolvedAddresses;
39-
import io.grpc.LoadBalancer.SubchannelPicker;
4037
import io.grpc.LoadBalancerProvider;
4138
import io.grpc.LoadBalancerRegistry;
4239
import io.grpc.Status;
@@ -55,7 +52,6 @@
5552
import org.junit.runner.RunWith;
5653
import org.junit.runners.JUnit4;
5754
import org.mockito.ArgumentCaptor;
58-
import org.mockito.ArgumentMatcher;
5955
import org.mockito.Captor;
6056
import org.mockito.Mock;
6157
import org.mockito.junit.MockitoJUnit;
@@ -224,42 +220,6 @@ private void deliverAddresses(WrrLocalityConfig config, List<EquivalentAddressGr
224220
.build());
225221
}
226222

227-
private static SubchannelPicker pickerReturns(final PickResult result) {
228-
return pickerReturns(new ArgumentMatcher<PickResult>() {
229-
@Override public boolean matches(PickResult obj) {
230-
return result.equals(obj);
231-
}
232-
233-
@Override public String toString() {
234-
return "[equals " + result + "]";
235-
}
236-
});
237-
}
238-
239-
private static SubchannelPicker pickerReturns(Status.Code code) {
240-
return pickerReturns(new ArgumentMatcher<PickResult>() {
241-
@Override public boolean matches(PickResult obj) {
242-
return obj.getStatus() != null && code.equals(obj.getStatus().getCode());
243-
}
244-
245-
@Override public String toString() {
246-
return "[with code " + code + "]";
247-
}
248-
});
249-
}
250-
251-
private static SubchannelPicker pickerReturns(final ArgumentMatcher<PickResult> matcher) {
252-
return argThat(new ArgumentMatcher<SubchannelPicker>() {
253-
@Override public boolean matches(SubchannelPicker picker) {
254-
return matcher.matches(picker.pickSubchannel(mock(PickSubchannelArgs.class)));
255-
}
256-
257-
@Override public String toString() {
258-
return "[picker returns: result " + matcher + "]";
259-
}
260-
});
261-
}
262-
263223
/**
264224
* Create a locality-labeled address.
265225
*/

0 commit comments

Comments
 (0)