Skip to content

Commit

Permalink
xds: Avoid switchTo in PriorityLb
Browse files Browse the repository at this point in the history
  • Loading branch information
@ejona86
ejona86 committed Jul 3, 2024
1 parent dfb22ba commit 0454b9e
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 76 deletions.
17 changes: 8 additions & 9 deletions xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import io.grpc.internal.BackoffPolicy;
import io.grpc.internal.ExponentialBackoffPolicy;
import io.grpc.internal.ObjectPool;
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import io.grpc.util.ForwardingLoadBalancerHelper;
import io.grpc.util.GracefulSwitchLoadBalancer;
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
Expand Down Expand Up @@ -726,8 +725,8 @@ private static PriorityChildConfig generateDnsBasedPriorityChildConfig(
dropOverloads, endpointLbConfig, tlsContext, filterMetadata);
LoadBalancerProvider clusterImplLbProvider =
lbRegistry.getProvider(XdsLbPolicies.CLUSTER_IMPL_POLICY_NAME);
PolicySelection clusterImplPolicy =
new PolicySelection(clusterImplLbProvider, clusterImplConfig);
Object clusterImplPolicy = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
clusterImplLbProvider, clusterImplConfig);
return new PriorityChildConfig(clusterImplPolicy, false /* ignoreReresolution*/);
}

Expand All @@ -751,15 +750,16 @@ private static Map<String, PriorityChildConfig> generateEdsBasedPriorityChildCon
dropOverloads, endpointLbConfig, tlsContext, filterMetadata);
LoadBalancerProvider clusterImplLbProvider =
lbRegistry.getProvider(XdsLbPolicies.CLUSTER_IMPL_POLICY_NAME);
PolicySelection priorityChildPolicy =
new PolicySelection(clusterImplLbProvider, clusterImplConfig);
Object priorityChildPolicy = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
clusterImplLbProvider, clusterImplConfig);

// If outlier detection has been configured we wrap the child policy in the outlier detection
// load balancer.
if (outlierDetection != null) {
LoadBalancerProvider outlierDetectionProvider = lbRegistry.getProvider(
"outlier_detection_experimental");
priorityChildPolicy = new PolicySelection(outlierDetectionProvider,
priorityChildPolicy = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
outlierDetectionProvider,
buildOutlierDetectionLbConfig(outlierDetection, priorityChildPolicy));
}

Expand All @@ -776,12 +776,11 @@ private static Map<String, PriorityChildConfig> generateEdsBasedPriorityChildCon
* understands.
*/
private static OutlierDetectionLoadBalancerConfig buildOutlierDetectionLbConfig(
OutlierDetection outlierDetection, PolicySelection childPolicy) {
OutlierDetection outlierDetection, Object childConfig) {
OutlierDetectionLoadBalancerConfig.Builder configBuilder
= new OutlierDetectionLoadBalancerConfig.Builder();

configBuilder.setChildConfig(GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
childPolicy.getProvider(), childPolicy.getConfig()));
configBuilder.setChildConfig(childConfig);

if (outlierDetection.intervalNanos() != null) {
configBuilder.setIntervalNanos(outlierDetection.intervalNanos());
Expand Down
12 changes: 1 addition & 11 deletions xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@
import io.grpc.ConnectivityState;
import io.grpc.InternalLogId;
import io.grpc.LoadBalancer;
import io.grpc.LoadBalancerProvider;
import io.grpc.Status;
import io.grpc.SynchronizationContext;
import io.grpc.SynchronizationContext.ScheduledHandle;
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import io.grpc.util.ForwardingLoadBalancerHelper;
import io.grpc.util.GracefulSwitchLoadBalancer;
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig;
Expand Down Expand Up @@ -208,7 +206,6 @@ private final class ChildLbState {
// Timer to delay shutdown and deletion of the priority. Scheduled whenever the child is
// deactivated.
@Nullable ScheduledHandle deletionTimer;
@Nullable String policy;
ConnectivityState connectivityState = CONNECTING;
SubchannelPicker picker = new FixedResultPicker(PickResult.withNoResult());

Expand Down Expand Up @@ -285,17 +282,10 @@ void tearDown() {
void updateResolvedAddresses() {
PriorityLbConfig config =
(PriorityLbConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
PolicySelection childPolicySelection = config.childConfigs.get(priority).policySelection;
LoadBalancerProvider lbProvider = childPolicySelection.getProvider();
String newPolicy = lbProvider.getPolicyName();
if (!newPolicy.equals(policy)) {
policy = newPolicy;
lb.switchTo(lbProvider);
}
lb.handleResolvedAddresses(
resolvedAddresses.toBuilder()
.setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), priority))
.setLoadBalancingPolicyConfig(childPolicySelection.getConfig())
.setLoadBalancingPolicyConfig(config.childConfigs.get(priority).childConfig)
.build());
}

Expand Down
9 changes: 4 additions & 5 deletions xds/src/main/java/io/grpc/xds/PriorityLoadBalancerProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import io.grpc.LoadBalancerProvider;
import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -90,18 +89,18 @@ public String toString() {
}

static final class PriorityChildConfig {
final PolicySelection policySelection;
final Object childConfig;
final boolean ignoreReresolution;

PriorityChildConfig(PolicySelection policySelection, boolean ignoreReresolution) {
this.policySelection = checkNotNull(policySelection, "policySelection");
PriorityChildConfig(Object childConfig, boolean ignoreReresolution) {
this.childConfig = checkNotNull(childConfig, "childConfig");
this.ignoreReresolution = ignoreReresolution;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("policySelection", policySelection)
.add("childConfig", childConfig)
.add("ignoreReresolution", ignoreReresolution)
.toString();
}
Expand Down
49 changes: 28 additions & 21 deletions xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,11 @@ public void edsClustersWithRingHashEndpointLbPolicy() {
PriorityChildConfig priorityChildConfig =
Iterables.getOnlyElement(priorityLbConfig.childConfigs.values());
assertThat(priorityChildConfig.ignoreReresolution).isTrue();
assertThat(priorityChildConfig.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig.childConfig)
.getPolicyName())
.isEqualTo(CLUSTER_IMPL_POLICY_NAME);
ClusterImplConfig clusterImplConfig =
(ClusterImplConfig) priorityChildConfig.policySelection.getConfig();
ClusterImplConfig clusterImplConfig = (ClusterImplConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig.childConfig);
assertClusterImplConfig(clusterImplConfig, CLUSTER1, EDS_SERVICE_NAME1, LRS_SERVER_INFO, 100L,
tlsContext, Collections.<DropOverload>emptyList(), "ring_hash_experimental");
RingHashConfig ringHashConfig = (RingHashConfig)
Expand Down Expand Up @@ -326,10 +327,11 @@ public void edsClustersWithLeastRequestEndpointLbPolicy() {
assertThat(priorityLbConfig.priorities).containsExactly(CLUSTER1 + "[child1]");
PriorityChildConfig priorityChildConfig =
Iterables.getOnlyElement(priorityLbConfig.childConfigs.values());
assertThat(priorityChildConfig.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig.childConfig)
.getPolicyName())
.isEqualTo(CLUSTER_IMPL_POLICY_NAME);
ClusterImplConfig clusterImplConfig =
(ClusterImplConfig) priorityChildConfig.policySelection.getConfig();
ClusterImplConfig clusterImplConfig = (ClusterImplConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig.childConfig);
assertClusterImplConfig(clusterImplConfig, CLUSTER1, EDS_SERVICE_NAME1, LRS_SERVER_INFO, 100L,
tlsContext, Collections.<DropOverload>emptyList(), WRR_LOCALITY_POLICY_NAME);
WrrLocalityConfig wrrLocalityConfig = (WrrLocalityConfig)
Expand Down Expand Up @@ -373,10 +375,11 @@ public void edsClustersWithOutlierDetection() {
Iterables.getOnlyElement(priorityLbConfig.childConfigs.values());

// The child config for priority should be outlier detection.
assertThat(priorityChildConfig.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig.childConfig)
.getPolicyName())
.isEqualTo("outlier_detection_experimental");
OutlierDetectionLoadBalancerConfig outlierDetectionConfig =
(OutlierDetectionLoadBalancerConfig) priorityChildConfig.policySelection.getConfig();
OutlierDetectionLoadBalancerConfig outlierDetectionConfig = (OutlierDetectionLoadBalancerConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig.childConfig);

// The outlier detection config should faithfully represent what came down from xDS.
assertThat(outlierDetectionConfig.intervalNanos).isEqualTo(outlierDetection.intervalNanos());
Expand Down Expand Up @@ -480,10 +483,11 @@ public void onlyEdsClusters_receivedEndpoints() {

PriorityChildConfig priorityChildConfig1 = priorityLbConfig.childConfigs.get(priority1);
assertThat(priorityChildConfig1.ignoreReresolution).isTrue();
assertThat(priorityChildConfig1.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig1.childConfig)
.getPolicyName())
.isEqualTo(CLUSTER_IMPL_POLICY_NAME);
ClusterImplConfig clusterImplConfig1 =
(ClusterImplConfig) priorityChildConfig1.policySelection.getConfig();
ClusterImplConfig clusterImplConfig1 = (ClusterImplConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig1.childConfig);
assertClusterImplConfig(clusterImplConfig1, CLUSTER2, EDS_SERVICE_NAME2, LRS_SERVER_INFO, 200L,
tlsContext, Collections.<DropOverload>emptyList(), WRR_LOCALITY_POLICY_NAME);
WrrLocalityConfig wrrLocalityConfig1 = (WrrLocalityConfig)
Expand All @@ -494,10 +498,11 @@ public void onlyEdsClusters_receivedEndpoints() {

PriorityChildConfig priorityChildConfig2 = priorityLbConfig.childConfigs.get(priority2);
assertThat(priorityChildConfig2.ignoreReresolution).isTrue();
assertThat(priorityChildConfig2.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig2.childConfig)
.getPolicyName())
.isEqualTo(CLUSTER_IMPL_POLICY_NAME);
ClusterImplConfig clusterImplConfig2 =
(ClusterImplConfig) priorityChildConfig2.policySelection.getConfig();
ClusterImplConfig clusterImplConfig2 = (ClusterImplConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig2.childConfig);
assertClusterImplConfig(clusterImplConfig2, CLUSTER2, EDS_SERVICE_NAME2, LRS_SERVER_INFO, 200L,
tlsContext, Collections.<DropOverload>emptyList(), WRR_LOCALITY_POLICY_NAME);
WrrLocalityConfig wrrLocalityConfig2 = (WrrLocalityConfig)
Expand All @@ -508,10 +513,11 @@ public void onlyEdsClusters_receivedEndpoints() {

PriorityChildConfig priorityChildConfig3 = priorityLbConfig.childConfigs.get(priority3);
assertThat(priorityChildConfig3.ignoreReresolution).isTrue();
assertThat(priorityChildConfig3.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig3.childConfig)
.getPolicyName())
.isEqualTo(CLUSTER_IMPL_POLICY_NAME);
ClusterImplConfig clusterImplConfig3 =
(ClusterImplConfig) priorityChildConfig3.policySelection.getConfig();
ClusterImplConfig clusterImplConfig3 = (ClusterImplConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig3.childConfig);
assertClusterImplConfig(clusterImplConfig3, CLUSTER1, EDS_SERVICE_NAME1, LRS_SERVER_INFO, 100L,
tlsContext, Collections.<DropOverload>emptyList(), WRR_LOCALITY_POLICY_NAME);
WrrLocalityConfig wrrLocalityConfig3 = (WrrLocalityConfig)
Expand Down Expand Up @@ -779,10 +785,11 @@ public void onlyLogicalDnsCluster_endpointsResolved() {
String priority = Iterables.getOnlyElement(priorityLbConfig.priorities);
PriorityChildConfig priorityChildConfig = priorityLbConfig.childConfigs.get(priority);
assertThat(priorityChildConfig.ignoreReresolution).isFalse();
assertThat(priorityChildConfig.policySelection.getProvider().getPolicyName())
assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider(priorityChildConfig.childConfig)
.getPolicyName())
.isEqualTo(CLUSTER_IMPL_POLICY_NAME);
ClusterImplConfig clusterImplConfig =
(ClusterImplConfig) priorityChildConfig.policySelection.getConfig();
ClusterImplConfig clusterImplConfig = (ClusterImplConfig)
GracefulSwitchLoadBalancerAccessor.getChildConfig(priorityChildConfig.childConfig);
assertClusterImplConfig(clusterImplConfig, CLUSTER_DNS, null, LRS_SERVER_INFO, 300L, null,
Collections.<DropOverload>emptyList(), "pick_first");
assertAddressesEqual(Arrays.asList(endpoint1, endpoint2), childBalancer.addresses);
Expand Down
10 changes: 7 additions & 3 deletions xds/src/test/java/io/grpc/xds/PriorityLoadBalancerProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import io.grpc.LoadBalancerProvider;
import io.grpc.internal.ServiceConfigUtil.PolicySelection;
import io.grpc.util.GracefulSwitchLoadBalancer;
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig;
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig;
import java.util.List;
Expand All @@ -45,7 +45,7 @@ public void priorityLbConfig_emptyPriorities() {
ImmutableMap.of(
"p0",
new PriorityChildConfig(
new PolicySelection(mock(LoadBalancerProvider.class), null), true));
newChildConfig(mock(LoadBalancerProvider.class), null), true));
List<String> priorities = ImmutableList.of();

thrown.expect(IllegalArgumentException.class);
Expand All @@ -59,10 +59,14 @@ public void priorityLbConfig_missingChildConfig() {
ImmutableMap.of(
"p1",
new PriorityChildConfig(
new PolicySelection(mock(LoadBalancerProvider.class), null), true));
newChildConfig(mock(LoadBalancerProvider.class), null), true));
List<String> priorities = ImmutableList.of("p0", "p1");

thrown.expect(IllegalArgumentException.class);
new PriorityLbConfig(childConfigs, priorities);
}

private Object newChildConfig(LoadBalancerProvider provider, Object config) {
return GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(provider, config);
}
}
Loading

0 comments on commit 0454b9e

Please sign in to comment.