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

Revert "Fix RLS regressions from XdsDepMan conversion" 

This reverts commit f6ba7c5291 as part of
reverting 297ab05efe.
This commit is contained in:
Eric Anderson 2025-07-16 11:18:19 -07:00
parent f6ba7c5291
commit 2ecbd437c3
5 changed files with 25 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java
View File

@ -255,13 +255,6 @@ final class CachingRlsLbClient {
}
}
Status acceptResolvedAddressFactory(ResolvedAddressFactory childLbResolvedAddressFactory) {
synchronized (lock) {
return refCountedChildPolicyWrapperFactory.acceptResolvedAddressFactory(
childLbResolvedAddressFactory);
}
}
/**
* Convert the status to UNAVAILABLE and enhance the error message.
* @param status status as provided by server

28
rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java
View File

@ -31,7 +31,6 @@ import io.grpc.LoadBalancer.SubchannelPicker;
import io.grpc.LoadBalancerProvider;
import io.grpc.LoadBalancerRegistry;
import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.ObjectPool;
import io.grpc.rls.ChildLoadBalancerHelper.ChildLoadBalancerHelperProvider;
import io.grpc.rls.RlsProtoData.RouteLookupConfig;
@ -212,7 +211,7 @@ final class LbPolicyConfiguration {
private final ChildLoadBalancerHelperProvider childLbHelperProvider;
private final ChildLbStatusListener childLbStatusListener;
private final ChildLoadBalancingPolicy childPolicy;
private ResolvedAddressFactory childLbResolvedAddressFactory;
private final ResolvedAddressFactory childLbResolvedAddressFactory;
public RefCountedChildPolicyWrapperFactory(
ChildLoadBalancingPolicy childPolicy,
@ -230,19 +229,6 @@ final class LbPolicyConfiguration {
childLbHelperProvider.init();
}
Status acceptResolvedAddressFactory(ResolvedAddressFactory childLbResolvedAddressFactory) {
this.childLbResolvedAddressFactory = childLbResolvedAddressFactory;
Status status = Status.OK;
for (RefCountedChildPolicyWrapper wrapper : childPolicyMap.values()) {
Status newStatus =
wrapper.childPolicyWrapper.acceptResolvedAddressFactory(childLbResolvedAddressFactory);
if (!newStatus.isOk()) {
status = newStatus;
}
}
return status;
}
ChildPolicyWrapper createOrGet(String target) {
// TODO(creamsoup) check if the target is valid or not
RefCountedChildPolicyWrapper pooledChildPolicyWrapper = childPolicyMap.get(target);
@ -291,7 +277,6 @@ final class LbPolicyConfiguration {
private final String target;
private final ChildPolicyReportingHelper helper;
private final LoadBalancer lb;
private final Object childLbConfig;
private volatile SubchannelPicker picker;
private ConnectivityState state;
@ -310,14 +295,14 @@ final class LbPolicyConfiguration {
.parseLoadBalancingPolicyConfig(
childPolicy.getEffectiveChildPolicy(target));
this.lb = lbProvider.newLoadBalancer(helper);
this.childLbConfig = lbConfig.getConfig();
helper.getChannelLogger().log(
ChannelLogLevel.DEBUG, "RLS child lb created. config: {0}", childLbConfig);
ChannelLogLevel.DEBUG, "RLS child lb created. config: {0}", lbConfig.getConfig());
helper.getSynchronizationContext().execute(
new Runnable() {
@Override
public void run() {
if (!acceptResolvedAddressFactory(childLbResolvedAddressFactory).isOk()) {
if (!lb.acceptResolvedAddresses(
childLbResolvedAddressFactory.create(lbConfig.getConfig())).isOk()) {
helper.refreshNameResolution();
}
lb.requestConnection();
@ -325,11 +310,6 @@ final class LbPolicyConfiguration {
});
}
Status acceptResolvedAddressFactory(ResolvedAddressFactory childLbResolvedAddressFactory) {
helper.getSynchronizationContext().throwIfNotInThisSynchronizationContext();
return lb.acceptResolvedAddresses(childLbResolvedAddressFactory.create(childLbConfig));
}
String getTarget() {
return target;
}

4
rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java
View File

@ -79,9 +79,7 @@ final class RlsLoadBalancer extends LoadBalancer {
// not required.
this.lbPolicyConfiguration = lbPolicyConfiguration;
}
return routeLookupClient.acceptResolvedAddressFactory(
new ChildLbResolvedAddressFactory(
resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()));
return Status.OK;
}
@Override

6
xds/src/main/java/io/grpc/xds/XdsNameResolver.java
View File

@ -823,9 +823,6 @@ final class XdsNameResolver extends NameResolver {
if (shouldUpdateResult && routingConfig != null) {
updateResolutionResult(xdsConfig);
shouldUpdateResult = false;
} else {
// Need to update at least once
shouldUpdateResult = true;
}
// Make newly added clusters selectable by config selector and deleted clusters no longer
// selectable.
@ -996,8 +993,7 @@ final class XdsNameResolver extends NameResolver {
.put("routeLookupConfig", rlsPluginConfig.config())
.put(
"childPolicy",
ImmutableList.of(ImmutableMap.of(XdsLbPolicies.CDS_POLICY_NAME, ImmutableMap.of(
"is_dynamic", true))))
ImmutableList.of(ImmutableMap.of(XdsLbPolicies.CDS_POLICY_NAME, ImmutableMap.of())))
.put("childPolicyConfigTargetFieldName", "cluster")
.buildOrThrow();
return ImmutableMap.of("rls_experimental", rlsConfig);

56
xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
View File

@ -28,7 +28,6 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@ -450,7 +449,7 @@ public class XdsNameResolverTest {
// Two new service config updates triggered:
// - with load balancing config being able to select cluster1 and cluster2
// - with load balancing config being able to select cluster2 only
verify(mockListener, times(3)).onResult2(resultCaptor.capture());
verify(mockListener, times(2)).onResult2(resultCaptor.capture());
assertServiceConfigForLoadBalancingConfig(
Arrays.asList(cluster1, cluster2),
(Map<String, ?>) resultCaptor.getAllValues().get(0).getServiceConfig().getConfig());
@ -1071,9 +1070,7 @@ public class XdsNameResolverTest {
assertCallSelectClusterResult(call1, configSelector, "another-cluster", 20.0);
firstCall.deliverErrorStatus(); // completes previous call
// Two updates: one for XdsNameResolver releasing the cluster, and another for
// XdsDependencyManager updating the XdsConfig
verify(mockListener, times(3)).onResult2(resolutionResultCaptor.capture());
verify(mockListener, times(2)).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
assertServiceConfigForLoadBalancingConfig(
Arrays.asList(cluster2, "another-cluster"),
@ -1103,7 +1100,7 @@ public class XdsNameResolverTest {
ImmutableMap.of())));
// Two consecutive service config updates: one for removing clcuster1,
// one for adding "another=cluster".
verify(mockListener, times(3)).onResult2(resolutionResultCaptor.capture());
verify(mockListener, times(2)).onResult2(resolutionResultCaptor.capture());
ResolutionResult result = resolutionResultCaptor.getValue();
assertServiceConfigForLoadBalancingConfig(
Arrays.asList(cluster2, "another-cluster"),
@ -1158,7 +1155,7 @@ public class XdsNameResolverTest {
cluster2, Collections.emptyList(),
TimeUnit.SECONDS.toNanos(15L), null, false),
ImmutableMap.of())));
verify(mockListener, times(2)).onResult2(resolutionResultCaptor.capture());
verifyNoMoreInteractions(mockListener); // no cluster added/deleted
assertCallSelectClusterResult(call1, configSelector, "another-cluster", 15.0);
}
@ -1190,13 +1187,7 @@ public class XdsNameResolverTest {
null, false),
ImmutableMap.of())));
testCall.deliverErrorStatus();
verify(mockListener, times(3)).onResult2(resolutionResultCaptor.capture());
assertServiceConfigForLoadBalancingConfig(
Arrays.asList(cluster1, cluster2), resolutionResultCaptor.getAllValues().get(1));
assertServiceConfigForLoadBalancingConfig(
Arrays.asList(cluster1, cluster2), resolutionResultCaptor.getAllValues().get(2));
assertServiceConfigForLoadBalancingConfig(
Arrays.asList(cluster1, cluster2), resolutionResultCaptor.getAllValues().get(3));
verifyNoMoreInteractions(mockListener);
}
@SuppressWarnings("unchecked")
@ -1277,7 +1268,7 @@ public class XdsNameResolverTest {
"routeLookupConfig",
ImmutableMap.of("lookupService", "rls-cbt.googleapis.com"),
"childPolicy",
ImmutableList.of(ImmutableMap.of("cds_experimental", ImmutableMap.of("is_dynamic", true))),
ImmutableList.of(ImmutableMap.of("cds_experimental", ImmutableMap.of())),
"childPolicyConfigTargetFieldName",
"cluster");
Map<String, ?> expectedClusterManagerLbConfig = ImmutableMap.of(
@ -1324,7 +1315,7 @@ public class XdsNameResolverTest {
"routeLookupConfig",
ImmutableMap.of("lookupService", "rls-cbt-2.googleapis.com"),
"childPolicy",
ImmutableList.of(ImmutableMap.of("cds_experimental", ImmutableMap.of("is_dynamic", true))),
ImmutableList.of(ImmutableMap.of("cds_experimental", ImmutableMap.of())),
"childPolicyConfigTargetFieldName",
"cluster");
Map<String, ?> expectedClusterManagerLbConfig2 = ImmutableMap.of(
@ -1665,7 +1656,7 @@ public class XdsNameResolverTest {
}
private void assertClusterResolutionResult(CallInfo call, String expectedCluster) {
verify(mockListener, atLeast(1)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
ResolutionResult result = resolutionResultCaptor.getValue();
InternalConfigSelector configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
assertCallSelectClusterResult(call, configSelector, expectedCluster, null);
@ -1753,13 +1744,6 @@ public class XdsNameResolverTest {
return result.getAttributes().get(InternalConfigSelector.KEY);
}
private static void assertServiceConfigForLoadBalancingConfig(
List<String> clusters, ResolutionResult result) {
@SuppressWarnings("unchecked")
Map<String, ?> config = (Map<String, ?>) result.getServiceConfig().getConfig();
assertServiceConfigForLoadBalancingConfig(clusters, config);
}
/**
* Verifies the raw service config contains an xDS load balancing config for the given clusters.
*/
@ -1863,9 +1847,7 @@ public class XdsNameResolverTest {
+ " \"lookupService\": \"rls.bigtable.google.com\"\n"
+ " },\n"
+ " \"childPolicy\": [\n"
+ " {\"cds_experimental\": {\n"
+ " \"is_dynamic\": true\n"
+ " }}\n"
+ " {\"cds_experimental\": {}}\n"
+ " ],\n"
+ " \"childPolicyConfigTargetFieldName\": \"cluster\"\n"
+ " }\n"
@ -2053,7 +2035,7 @@ public class XdsNameResolverTest {
FaultAbort.forHeader(FaultConfig.FractionalPercent.perMillion(600_000)),
null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(2)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2069,7 +2051,7 @@ public class XdsNameResolverTest {
FaultAbort.forHeader(FaultConfig.FractionalPercent.perMillion(0)),
null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(3)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2084,7 +2066,7 @@ public class XdsNameResolverTest {
FaultConfig.FractionalPercent.perMillion(600_000)),
null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(4)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2102,7 +2084,7 @@ public class XdsNameResolverTest {
FaultConfig.FractionalPercent.perMillion(400_000)),
null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(5)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2137,7 +2119,7 @@ public class XdsNameResolverTest {
httpFilterFaultConfig = FaultConfig.create(
FaultDelay.forHeader(FaultConfig.FractionalPercent.perMillion(600_000)), null, null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(2)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2148,7 +2130,7 @@ public class XdsNameResolverTest {
httpFilterFaultConfig = FaultConfig.create(
FaultDelay.forHeader(FaultConfig.FractionalPercent.perMillion(0)), null, null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(3)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2161,7 +2143,7 @@ public class XdsNameResolverTest {
null,
null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(4)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2174,7 +2156,7 @@ public class XdsNameResolverTest {
null,
null);
xdsClient.deliverLdsUpdateWithFaultInjection(cluster1, httpFilterFaultConfig, null, null, null);
verify(mockListener, times(5)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2299,7 +2281,7 @@ public class XdsNameResolverTest {
null);
xdsClient.deliverLdsUpdateWithFaultInjection(
cluster1, httpFilterFaultConfig, virtualHostFaultConfig, routeFaultConfig, null);
verify(mockListener, times(2)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,
@ -2316,7 +2298,7 @@ public class XdsNameResolverTest {
xdsClient.deliverLdsUpdateWithFaultInjection(
cluster1, httpFilterFaultConfig, virtualHostFaultConfig, routeFaultConfig,
weightedClusterFaultConfig);
verify(mockListener, times(3)).onResult2(resolutionResultCaptor.capture());
verify(mockListener).onResult2(resolutionResultCaptor.capture());
result = resolutionResultCaptor.getValue();
configSelector = result.getAttributes().get(InternalConfigSelector.KEY);
observer = startNewCall(TestMethodDescriptors.voidMethod(), configSelector,