Skip to content

xds: Avoid changing cache when watching children in XdsDepManager #12138

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.

Sign up for GitHub

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 1 commit into from
Jun 13, 2025
Merged

xds: Avoid changing cache when watching children in XdsDepManager #12138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
74 changes: 22 additions & 52 deletions xds/src/main/java/io/grpc/xds/XdsDependencyManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@
Status.INTERNAL.withDescription("Logical DNS in dependency manager unsupported")));
break;
default:
throw new IllegalStateException("Unexpected value: " + cdsUpdate.clusterType());
child = new EndpointConfig(StatusOr.fromStatus(Status.UNAVAILABLE.withDescription(
"Unknown type in cluster " + clusterName + " " + cdsUpdate.clusterType())));

Check warning on line 327 in xds/src/main/java/io/grpc/xds/XdsDependencyManager.java

View check run for this annotation

Codecov / codecov/patch

xds/src/main/java/io/grpc/xds/XdsDependencyManager.java#L326-L327 

Added lines #L326 - L327 were not covered by tests
}
clusters.put(clusterName, StatusOr.fromValue(
new XdsConfig.XdsClusterConfig(clusterName, cdsUpdate, child)));
Expand Down Expand Up @@ -505,7 +506,7 @@
}
// Don't update configuration on error, if we've already received configuration
if (!hasDataValue()) {
setDataAsStatus(Status.UNAVAILABLE.withDescription(
this.data = StatusOr.fromStatus(Status.UNAVAILABLE.withDescription(
String.format("Error retrieving %s: %s: %s",
toContextString(), error.getCode(), error.getDescription())));
maybePublishConfig();
Expand All @@ -519,11 +520,25 @@
}

checkArgument(this.resourceName.equals(resourceName), "Resource name does not match");
setDataAsStatus(Status.UNAVAILABLE.withDescription(
this.data = StatusOr.fromStatus(Status.UNAVAILABLE.withDescription(
toContextString() + " does not exist" + nodeInfo()));
maybePublishConfig();
}

@Override
public void onChanged(T update) {
checkNotNull(update, "update");
if (cancelled) {
return;
}

this.data = StatusOr.fromValue(update);
subscribeToChildren(update);
maybePublishConfig();
}

protected abstract void subscribeToChildren(T update);

public void close() {
cancelled = true;
xdsClient.cancelXdsResourceWatch(type, resourceName, this);
Expand All @@ -542,20 +557,6 @@
return data != null && data.hasValue();
}

String resourceName() {
return resourceName;
}

protected void setData(T data) {
checkNotNull(data, "data");
this.data = StatusOr.fromValue(data);
}

protected void setDataAsStatus(Status status) {
checkNotNull(status, "status");
this.data = StatusOr.fromStatus(status);
}

public String toContextString() {
return toContextStr(type.typeName(), resourceName);
}
Expand All @@ -573,12 +574,7 @@
}

@Override
public void onChanged(XdsListenerResource.LdsUpdate update) {
checkNotNull(update, "update");
if (cancelled) {
return;
}

public void subscribeToChildren(XdsListenerResource.LdsUpdate update) {
HttpConnectionManager httpConnectionManager = update.httpConnectionManager();
List<VirtualHost> virtualHosts;
if (httpConnectionManager == null) {
Expand All @@ -595,9 +591,6 @@
if (rdsName != null) {
addRdsWatcher(rdsName);
}

setData(update);
maybePublishConfig();
}

private String getRdsName(XdsListenerResource.LdsUpdate update) {
Expand Down Expand Up @@ -665,14 +658,8 @@
}

@Override
public void onChanged(RdsUpdate update) {
checkNotNull(update, "update");
if (cancelled) {
return;
}
setData(update);
public void subscribeToChildren(RdsUpdate update) {
updateRoutes(update.virtualHosts);
maybePublishConfig();
}

@Override
Expand All @@ -690,31 +677,20 @@
}

@Override
public void onChanged(XdsClusterResource.CdsUpdate update) {
checkNotNull(update, "update");
if (cancelled) {
return;
}
public void subscribeToChildren(XdsClusterResource.CdsUpdate update) {
switch (update.clusterType()) {
case EDS:
setData(update);
addEdsWatcher(getEdsServiceName());
break;
case LOGICAL_DNS:
setData(update);
// no eds needed
break;
case AGGREGATE:
setData(update);
update.prioritizedClusterNames()
.forEach(name -> addClusterWatcher(name));
break;
default:
Status error = Status.UNAVAILABLE.withDescription(
"unknown cluster type in " + resourceName() + " " + update.clusterType());
setDataAsStatus(error);
}
maybePublishConfig();
}

public String getEdsServiceName() {
Expand All @@ -734,12 +710,6 @@
}

@Override
public void onChanged(XdsEndpointResource.EdsUpdate update) {
if (cancelled) {
return;
}
setData(checkNotNull(update, "update"));
maybePublishConfig();
}
public void subscribeToChildren(XdsEndpointResource.EdsUpdate update) {}
}
}