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

api: Deprecate LoadBalancer.EMPTY_PICKER 

FixedResultPicker is our preferred approach.
This commit is contained in:
Eric Anderson 2023-10-19 15:59:28 -07:00
parent 3b92333890
commit 860b5cb1f0
10 changed files with 100 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api/src/main/java/io/grpc/LoadBalancer.java
View File

@ -116,6 +116,12 @@ public abstract class LoadBalancer {
public static final Attributes.Key<Map<String, ?>> ATTR_HEALTH_CHECKING_CONFIG =
Attributes.Key.create("internal:health-checking-config");
/**
* A picker that always returns an erring pick.
*
* @deprecated Use {@code new FixedResultPicker(PickResult.withNoResult())} instead.
*/
@Deprecated
public static final SubchannelPicker EMPTY_PICKER = new SubchannelPicker() {
@Override
public PickResult pickSubchannel(PickSubchannelArgs args) {

68
api/src/testFixtures/java/io/grpc/LoadBalancerMatchers.java Normal file
View File

@ -0,0 +1,68 @@
/*
* Copyright 2023 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.grpc;
import static org.mockito.Mockito.argThat;
import static org.mockito.Mockito.mock;
import io.grpc.LoadBalancer.PickResult;
import io.grpc.LoadBalancer.PickSubchannelArgs;
import io.grpc.LoadBalancer.SubchannelPicker;
import org.mockito.ArgumentMatcher;
/**
* Mockito matchers for testing LoadBalancers.
*/
public final class LoadBalancerMatchers {
private LoadBalancerMatchers() {}
public static SubchannelPicker pickerReturns(final PickResult result) {
return pickerReturns(new ArgumentMatcher<PickResult>() {
@Override public boolean matches(PickResult obj) {
return result.equals(obj);
}
@Override public String toString() {
return "[equals " + result + "]";
}
});
}
public static SubchannelPicker pickerReturns(Status.Code code) {
return pickerReturns(new ArgumentMatcher<PickResult>() {
@Override public boolean matches(PickResult obj) {
return obj.getStatus() != null && code.equals(obj.getStatus().getCode());
}
@Override public String toString() {
return "[with code " + code + "]";
}
});
}
public static SubchannelPicker pickerReturns(final ArgumentMatcher<PickResult> matcher) {
return argThat(new ArgumentMatcher<SubchannelPicker>() {
@Override public boolean matches(SubchannelPicker picker) {
return matcher.matches(picker.pickSubchannel(mock(PickSubchannelArgs.class)));
}
@Override public String toString() {
return "[picker returns: result " + matcher + "]";
}
});
}
}

2
util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java
View File

@ -73,7 +73,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
Map<Object, SubchannelPicker> childPickers);
protected SubchannelPicker getInitialPicker() {
return EMPTY_PICKER;
return new FixedResultPicker(PickResult.withNoResult());
}
protected SubchannelPicker getErrorPicker(Status error) {

1
xds/build.gradle
View File

@ -59,6 +59,7 @@ dependencies {
testImplementation project(':grpc-rls')
testImplementation testFixtures(project(':grpc-core')),
testFixtures(project(':grpc-api')),
testFixtures(project(':grpc-util'))
annotationProcessor libraries.auto.value

2
xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java
View File

@ -172,7 +172,7 @@ final class ClusterImplLoadBalancer extends LoadBalancer {
private final class ClusterImplLbHelper extends ForwardingLoadBalancerHelper {
private final AtomicLong inFlights;
private ConnectivityState currentState = ConnectivityState.IDLE;
private SubchannelPicker currentPicker = LoadBalancer.EMPTY_PICKER;
private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult());
private List<DropOverload> dropPolicies = Collections.emptyList();
private long maxConcurrentRequests = DEFAULT_PER_CLUSTER_MAX_CONCURRENT_REQUESTS;
@Nullable

4
xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java
View File

@ -148,7 +148,7 @@ final class PriorityLoadBalancer extends LoadBalancer {
ChildLbState child =
new ChildLbState(priority, priorityConfigs.get(priority).ignoreReresolution);
children.put(priority, child);
updateOverallState(priority, CONNECTING, LoadBalancer.EMPTY_PICKER);
updateOverallState(priority, CONNECTING, new FixedResultPicker(PickResult.withNoResult()));
// Calling the child's updateResolvedAddresses() can result in tryNextPriority() being
// called recursively. We need to be sure to be done with processing here before it is
// called.
@ -209,7 +209,7 @@ final class PriorityLoadBalancer extends LoadBalancer {
@Nullable ScheduledHandle deletionTimer;
@Nullable String policy;
ConnectivityState connectivityState = CONNECTING;
SubchannelPicker picker = LoadBalancer.EMPTY_PICKER;
SubchannelPicker picker = new FixedResultPicker(PickResult.withNoResult());
ChildLbState(final String priority, boolean ignoreReresolution) {
this.priority = priority;

4
xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java
View File

@ -158,7 +158,7 @@ final class WeightedTargetLoadBalancer extends LoadBalancer {
if (overallState == TRANSIENT_FAILURE) {
picker = new WeightedRandomPicker(errorPickers);
} else {
picker = LoadBalancer.EMPTY_PICKER;
picker = new FixedResultPicker(PickResult.withNoResult());
}
} else {
picker = new WeightedRandomPicker(childPickers);
@ -190,7 +190,7 @@ final class WeightedTargetLoadBalancer extends LoadBalancer {
private final class ChildHelper extends ForwardingLoadBalancerHelper {
String name;
ConnectivityState currentState = CONNECTING;
SubchannelPicker currentPicker = LoadBalancer.EMPTY_PICKER;
SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult());
private ChildHelper(String name) {
this.name = name;

11
xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java
View File

@ -21,7 +21,7 @@ import static io.grpc.ConnectivityState.CONNECTING;
import static io.grpc.ConnectivityState.IDLE;
import static io.grpc.ConnectivityState.READY;
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
import static io.grpc.LoadBalancer.EMPTY_PICKER;
import static io.grpc.LoadBalancerMatchers.pickerReturns;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
@ -76,6 +76,9 @@ import org.mockito.junit.MockitoRule;
/** Tests for {@link PriorityLoadBalancer}. */
@RunWith(JUnit4.class)
public class PriorityLoadBalancerTest {
private static final SubchannelPicker EMPTY_PICKER
= new FixedResultPicker(PickResult.withNoResult());
private final List<LoadBalancer> fooBalancers = new ArrayList<>();
private final List<LoadBalancer> barBalancers = new ArrayList<>();
private final List<Helper> fooHelpers = new ArrayList<>();
@ -457,6 +460,9 @@ public class PriorityLoadBalancerTest {
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(priorityLbConfig)
.build());
// Nothing important about this verify, other than to provide a baseline
verify(helper, times(2))
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
assertThat(fooBalancers).hasSize(1);
assertThat(fooHelpers).hasSize(1);
Helper helper0 = Iterables.getOnlyElement(fooHelpers);
@ -472,7 +478,8 @@ public class PriorityLoadBalancerTest {
helper0.updateBalancingState(
CONNECTING,
EMPTY_PICKER);
verify(helper, times(2)).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper, times(3))
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
// failover happens
fakeClock.forwardTime(10, TimeUnit.SECONDS);

16
xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java
View File

@ -20,7 +20,7 @@ import static com.google.common.truth.Truth.assertThat;
import static io.grpc.ConnectivityState.CONNECTING;
import static io.grpc.ConnectivityState.READY;
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
import static io.grpc.LoadBalancer.EMPTY_PICKER;
import static io.grpc.LoadBalancerMatchers.pickerReturns;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atLeast;
@ -209,7 +209,7 @@ public class WeightedTargetLoadBalancerTest {
.setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
.build());
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
assertThat(childBalancers).hasSize(4);
assertThat(childHelpers).hasSize(4);
assertThat(fooLbCreated).isEqualTo(2);
@ -246,7 +246,8 @@ public class WeightedTargetLoadBalancerTest {
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets))
.build());
verify(helper, atLeast(2)).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper, atLeast(2))
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
assertThat(childBalancers).hasSize(5);
assertThat(childHelpers).hasSize(5);
assertThat(fooLbCreated).isEqualTo(3); // One more foo LB created for target4
@ -288,7 +289,7 @@ public class WeightedTargetLoadBalancerTest {
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
.build());
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
// Error after child balancers created.
weightedTargetLb.handleNameResolutionError(Status.ABORTED);
@ -315,7 +316,7 @@ public class WeightedTargetLoadBalancerTest {
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
.build());
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
// Subchannels to be created for each child balancer.
final SubchannelPicker[] subchannelPickers = new SubchannelPicker[]{
@ -335,7 +336,8 @@ public class WeightedTargetLoadBalancerTest {
childHelpers.get(1).updateBalancingState(TRANSIENT_FAILURE, failurePickers[1]);
verify(helper, never()).updateBalancingState(
eq(TRANSIENT_FAILURE), any(SubchannelPicker.class));
verify(helper, times(2)).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper, times(2))
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
// Another child balancer goes to READY.
childHelpers.get(2).updateBalancingState(READY, subchannelPickers[2]);
@ -396,7 +398,7 @@ public class WeightedTargetLoadBalancerTest {
.setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
.build());
verify(helper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER));
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
// LB shutdown and subchannel state change can happen simultaneously. If shutdown runs first,
// any further balancing state update should be ignored.

42
xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java
View File

@ -17,11 +17,10 @@
package io.grpc.xds;
import static com.google.common.truth.Truth.assertThat;
import static io.grpc.LoadBalancerMatchers.pickerReturns;
import static io.grpc.xds.XdsLbPolicies.WEIGHTED_TARGET_POLICY_NAME;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.argThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ -34,9 +33,7 @@ import io.grpc.EquivalentAddressGroup;
import io.grpc.LoadBalancer;
import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancer.PickResult;
import io.grpc.LoadBalancer.PickSubchannelArgs;
import io.grpc.LoadBalancer.ResolvedAddresses;
import io.grpc.LoadBalancer.SubchannelPicker;
import io.grpc.LoadBalancerProvider;
import io.grpc.LoadBalancerRegistry;
import io.grpc.Status;
@ -55,7 +52,6 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
@ -224,42 +220,6 @@ public class WrrLocalityLoadBalancerTest {
.build());
}
private static SubchannelPicker pickerReturns(final PickResult result) {
return pickerReturns(new ArgumentMatcher<PickResult>() {
@Override public boolean matches(PickResult obj) {
return result.equals(obj);
}
@Override public String toString() {
return "[equals " + result + "]";
}
});
}
private static SubchannelPicker pickerReturns(Status.Code code) {
return pickerReturns(new ArgumentMatcher<PickResult>() {
@Override public boolean matches(PickResult obj) {
return obj.getStatus() != null && code.equals(obj.getStatus().getCode());
}
@Override public String toString() {
return "[with code " + code + "]";
}
});
}
private static SubchannelPicker pickerReturns(final ArgumentMatcher<PickResult> matcher) {
return argThat(new ArgumentMatcher<SubchannelPicker>() {
@Override public boolean matches(SubchannelPicker picker) {
return matcher.matches(picker.pickSubchannel(mock(PickSubchannelArgs.class)));
}
@Override public String toString() {
return "[picker returns: result " + matcher + "]";
}
});
}
/**
* Create a locality-labeled address.
*/