Skip to content

xds: Revert CdsLb to XdsDepManager, and follow-up commits (v1.74.x) #12216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 0 additions & 27 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,6 @@ public static final class Args {
@Nullable private final Executor executor;
@Nullable private final String overrideAuthority;
@Nullable private final MetricRecorder metricRecorder;
@Nullable private final NameResolverRegistry nameResolverRegistry;
@Nullable private final IdentityHashMap<Key<?>, Object> customArgs;

private Args(Builder builder) {
Expand All @@ -317,7 +316,6 @@ private Args(Builder builder) {
this.executor = builder.executor;
this.overrideAuthority = builder.overrideAuthority;
this.metricRecorder = builder.metricRecorder;
this.nameResolverRegistry = builder.nameResolverRegistry;
this.customArgs = cloneCustomArgs(builder.customArgs);
}

Expand Down Expand Up @@ -449,18 +447,6 @@ public MetricRecorder getMetricRecorder() {
return metricRecorder;
}

/**
* Returns the {@link NameResolverRegistry} that the Channel uses to look for {@link
* NameResolver}s.
*
* @since 1.74.0
*/
public NameResolverRegistry getNameResolverRegistry() {
if (nameResolverRegistry == null) {
throw new IllegalStateException("NameResolverRegistry is not set in Builder");
}
return nameResolverRegistry;
}

@Override
public String toString() {
Expand All @@ -475,7 +461,6 @@ public String toString() {
.add("executor", executor)
.add("overrideAuthority", overrideAuthority)
.add("metricRecorder", metricRecorder)
.add("nameResolverRegistry", nameResolverRegistry)
.toString();
}

Expand All @@ -495,7 +480,6 @@ public Builder toBuilder() {
builder.setOffloadExecutor(executor);
builder.setOverrideAuthority(overrideAuthority);
builder.setMetricRecorder(metricRecorder);
builder.setNameResolverRegistry(nameResolverRegistry);
builder.customArgs = cloneCustomArgs(customArgs);
return builder;
}
Expand Down Expand Up @@ -524,7 +508,6 @@ public static final class Builder {
private Executor executor;
private String overrideAuthority;
private MetricRecorder metricRecorder;
private NameResolverRegistry nameResolverRegistry;
private IdentityHashMap<Key<?>, Object> customArgs;

Builder() {
Expand Down Expand Up @@ -631,16 +614,6 @@ public Builder setMetricRecorder(MetricRecorder metricRecorder) {
return this;
}

/**
* See {@link Args#getNameResolverRegistry}. This is an optional field.
*
* @since 1.74.0
*/
public Builder setNameResolverRegistry(NameResolverRegistry registry) {
this.nameResolverRegistry = registry;
return this;
}

/**
* Builds an {@link Args}.
*
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,7 @@ ClientStream newSubstream(
.setChannelLogger(channelLogger)
.setOffloadExecutor(this.offloadExecutorHolder)
.setOverrideAuthority(this.authorityOverride)
.setMetricRecorder(this.metricRecorder)
.setNameResolverRegistry(builder.nameResolverRegistry);
.setMetricRecorder(this.metricRecorder);
builder.copyAllNameResolverCustomArgsTo(nameResolverArgsBuilder);
this.nameResolverArgs = nameResolverArgsBuilder.build();
this.nameResolver = getNameResolver(
Expand Down Expand Up @@ -686,7 +685,11 @@ static NameResolver getNameResolver(
// We wrap the name resolver in a RetryingNameResolver to give it the ability to retry failures.
// TODO: After a transition period, all NameResolver implementations that need retry should use
// RetryingNameResolver directly and this step can be removed.
NameResolver usedNameResolver = RetryingNameResolver.wrap(resolver, nameResolverArgs);
NameResolver usedNameResolver = new RetryingNameResolver(resolver,
new BackoffPolicyRetryScheduler(new ExponentialBackoffPolicy.Provider(),
nameResolverArgs.getScheduledExecutorService(),
nameResolverArgs.getSynchronizationContext()),
nameResolverArgs.getSynchronizationContext());

if (overrideAuthority == null) {
return usedNameResolver;
Expand Down
13 changes: 2 additions & 11 deletions core/src/main/java/io/grpc/internal/RetryingNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,13 @@
*
* <p>The {@link NameResolver} used with this
*/
public final class RetryingNameResolver extends ForwardingNameResolver {
public static NameResolver wrap(NameResolver retriedNameResolver, Args args) {
// For migration, this might become conditional
return new RetryingNameResolver(
retriedNameResolver,
new BackoffPolicyRetryScheduler(
new ExponentialBackoffPolicy.Provider(),
args.getScheduledExecutorService(),
args.getSynchronizationContext()),
args.getSynchronizationContext());
}
final class RetryingNameResolver extends ForwardingNameResolver {

private final NameResolver retriedNameResolver;
private final RetryScheduler retryScheduler;
private final SynchronizationContext syncContext;


/**
* Creates a new {@link RetryingNameResolver}.
*
Expand Down
9 changes: 8 additions & 1 deletion core/src/test/java/io/grpc/internal/DnsNameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,14 @@ private RetryingNameResolver newResolver(

// In practice the DNS name resolver provider always wraps the resolver in a
// RetryingNameResolver which adds retry capabilities to it. We use the same setup here.
return (RetryingNameResolver) RetryingNameResolver.wrap(dnsResolver, args);
return new RetryingNameResolver(
dnsResolver,
new BackoffPolicyRetryScheduler(
new ExponentialBackoffPolicy.Provider(),
fakeExecutor.getScheduledExecutorService(),
syncContext
),
syncContext);
}

@Before
Expand Down
7 changes: 0 additions & 7 deletions rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -255,13 +255,6 @@ void init() {
}
}

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
Expand Down
28 changes: 4 additions & 24 deletions rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
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;
Expand Down Expand Up @@ -212,7 +211,7 @@ static final class RefCountedChildPolicyWrapperFactory {
private final ChildLoadBalancerHelperProvider childLbHelperProvider;
private final ChildLbStatusListener childLbStatusListener;
private final ChildLoadBalancingPolicy childPolicy;
private ResolvedAddressFactory childLbResolvedAddressFactory;
private final ResolvedAddressFactory childLbResolvedAddressFactory;

public RefCountedChildPolicyWrapperFactory(
ChildLoadBalancingPolicy childPolicy,
Expand All @@ -230,19 +229,6 @@ void init() {
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);
Expand Down Expand Up @@ -291,7 +277,6 @@ static final class ChildPolicyWrapper {
private final String target;
private final ChildPolicyReportingHelper helper;
private final LoadBalancer lb;
private final Object childLbConfig;
private volatile SubchannelPicker picker;
private ConnectivityState state;

Expand All @@ -310,26 +295,21 @@ public ChildPolicyWrapper(
.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();
}
});
}

Status acceptResolvedAddressFactory(ResolvedAddressFactory childLbResolvedAddressFactory) {
helper.getSynchronizationContext().throwIfNotInThisSynchronizationContext();
return lb.acceptResolvedAddresses(childLbResolvedAddressFactory.create(childLbConfig));
}

String getTarget() {
return target;
}
Expand Down
4 changes: 1 addition & 3 deletions rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
// not required.
this.lbPolicyConfiguration = lbPolicyConfiguration;
}
return routeLookupClient.acceptResolvedAddressFactory(
new ChildLbResolvedAddressFactory(
resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()));
return Status.OK;
}

@Override
Expand Down
Loading
Loading