Skip to content
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

Add new classes for eliminating xds config tears #11740

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
+1,969 −25
Open

Add new classes for eliminating xds config tears #11740

Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fd069e3
Framework definition to support A74
larry-safran Dec 4, 2024
b2cb05b
Test that update works (with associated fixes)
larry-safran Jan 6, 2025
8e668b5
Cleanup
larry-safran Jan 6, 2025
5b14a3e
Cleanup
larry-safran Jan 6, 2025
5f8d479
Add verification of data changing
larry-safran Jan 7, 2025
ec50490
Support aggregate clusters correctly
larry-safran Jan 7, 2025
5dceeaf
Fix class name referenced in javadoc
larry-safran Jan 7, 2025
fd64f20
Address a number of code review comments and add a test for missing C…
larry-safran Jan 8, 2025
b2e924e
Remove syncContext from watchers. Add checkNotNull, private and fina…
larry-safran Jan 8, 2025
5a75b10
Add a test for corrupt LDS
larry-safran Jan 8, 2025
d3b713f
Change aggregate cluster handling to correctly handle cluster names a…
larry-safran Jan 9, 2025
6089730
Errorprone
larry-safran Jan 9, 2025
d5dea83
Add max recursion limit for clusters to match c++.
larry-safran Jan 14, 2025
28d29fb
Fix handling of route and cluster updates.
larry-safran Jan 15, 2025
4a53fce
Make data private and XdsWatcherBase static
larry-safran Jan 15, 2025
c97118f
In LDS onChanged(), get old activeVirtualHost before possibly doing c…
larry-safran Jan 16, 2025
06466fc
Change comment for clarity
larry-safran Jan 16, 2025
b898e34
Allow EdsWatcher to have multiple CdsWatcher parents
larry-safran Jan 16, 2025
954ced3
Allow EdsWatcher to have multiple CdsWatcher parents
larry-safran Jan 16, 2025
ef13712
Fully support inlined RouteConfig
larry-safran Jan 17, 2025
518cef1
Add lots of `checkNotNull()`
larry-safran Jan 17, 2025
68071e6
Add test case testMultipleParentsInCdsTree, make a couple of cluster …
larry-safran Jan 18, 2025
f99fc56
Add test case testMultipleParentsInCdsTree, make a couple of cluster …
larry-safran Jan 18, 2025
482cd9d
Add virtual host to XdsConfig as per spec
larry-safran Jan 21, 2025
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
24 changes: 17 additions & 7 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 @@ -54,7 +54,7 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
public static final XdsClusterResource CLUSTER_RESOURCE = XdsClusterResource.getInstance();
public static final String TOP_CDS_CONTEXT = toContextStr(CLUSTER_RESOURCE.typeName(), "");
public static final XdsEndpointResource ENDPOINT_RESOURCE = XdsEndpointResource.getInstance();
public static final String CLUSTER_TYPE = XdsClusterResource.getInstance().typeName();
private static final int MAX_CLUSTER_RECURSION_DEPTH = 16; // Matches core
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/core/c core/ (or "C++")? Core here would typically mean grpc-core, the Java code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with C++

private final XdsClient xdsClient;
private final XdsConfigWatcher xdsConfigWatcher;
private final SynchronizationContext syncContext;
Expand Down Expand Up @@ -94,7 +94,7 @@ public Closeable subscribeToCluster(String clusterName) {
Set<ClusterSubscription> localSubscriptions =
clusterSubscriptions.computeIfAbsent(clusterName, k -> new HashSet<>());
localSubscriptions.add(subscription);
addWatcher(new CdsWatcher(clusterName, TOP_CDS_CONTEXT));
addWatcher(new CdsWatcher(clusterName, TOP_CDS_CONTEXT, 1));
});

return subscription;
Expand Down Expand Up @@ -485,11 +485,11 @@ List<String> getCdsNames() {
}

private class CdsWatcher extends XdsWatcherBase<XdsClusterResource.CdsUpdate> {
List<String> parentContexts = new ArrayList<>();
Map<String, Integer> parentContexts = new HashMap<>();

CdsWatcher(String resourceName, String parentContext) {
CdsWatcher(String resourceName, String parentContext, int depth) {
super(CLUSTER_RESOURCE, resourceName);
this.parentContexts.add(parentContext);
this.parentContexts.put(parentContext, depth);
}

@Override
Expand Down Expand Up @@ -518,11 +518,21 @@ public void onChanged(XdsClusterResource.CdsUpdate update) {

Set<String> addedClusters = Sets.difference(newNames, oldNames);
Set<String> deletedClusters = Sets.difference(oldNames, newNames);
addedClusters.forEach((cluster) -> addWatcher(new CdsWatcher(cluster, parentContext)));
int depth = parentContexts.values().stream().max(Integer::compare).orElse(0) + 1;
if (depth > MAX_CLUSTER_RECURSION_DEPTH) {
logger.log(XdsLogger.XdsLogLevel.WARNING,
"Cluster recursion depth limit exceeded for cluster {0}", resourceName());
Status error = Status.UNAVAILABLE.withDescription(
"aggregate cluster graph exceeds max depth");
data = StatusOr.fromStatus(error);
throw error.asRuntimeException();
ejona86 marked this conversation as resolved.
Show resolved Hide resolved
}
addedClusters.forEach(
(cluster) -> addWatcher(new CdsWatcher(cluster, parentContext, depth)));
deletedClusters.forEach((cluster)
-> cancelClusterWatcherTree(getCluster(cluster), parentContext));

if (!addedClusters.isEmpty()) {
if (addedClusters.isEmpty()) {
maybePublishConfig();
}
} else {
Expand Down
Loading