Skip to content

Commit 48d08e6

Browse files
authored
xds: Remove EDS maybePublishConfig() avoidance in XdsDepManager (#12121)
The optimization makes the code more complicated. Yes, we know that maybePublishConfig() will do no work because of an outstanding watch, but we don't do this for other new watchers created and doing so would just make the code more bug-prone. This removes a difference in how different watcher types are handled.
1 parent 6bad600 commit 48d08e6

File tree

1 file changed

+4
-16
lines changed

1 file changed

+4
-16
lines changed

xds/src/main/java/io/grpc/xds/XdsDependencyManager.java

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -456,17 +456,15 @@ public String toString() {
456456
return logId.toString();
457457
}
458458

459-
// Returns true if the watcher was added, false if it already exists
460-
private boolean addEdsWatcher(String edsServiceName, CdsWatcher parentContext) {
459+
private void addEdsWatcher(String edsServiceName, CdsWatcher parentContext) {
461460
EdsWatcher watcher
462461
= (EdsWatcher) getWatchers(XdsEndpointResource.getInstance()).get(edsServiceName);
463462
if (watcher != null) {
464463
watcher.addParentContext(parentContext); // Is a set, so don't need to check for existence
465-
return false;
464+
return;
466465
}
467466

468467
addWatcher(new EdsWatcher(edsServiceName, parentContext));
469-
return true;
470468
}
471469

472470
private void addClusterWatcher(String clusterName, Object parentContext, int depth) {
@@ -823,13 +821,10 @@ public void onChanged(XdsClusterResource.CdsUpdate update) {
823821
switch (update.clusterType()) {
824822
case EDS:
825823
setData(update);
826-
if (!addEdsWatcher(getEdsServiceName(), this)) {
827-
maybePublishConfig();
828-
}
824+
addEdsWatcher(getEdsServiceName(), this);
829825
break;
830826
case LOGICAL_DNS:
831827
setData(update);
832-
maybePublishConfig();
833828
// no eds needed
834829
break;
835830
case AGGREGATE:
@@ -856,27 +851,20 @@ public void onChanged(XdsClusterResource.CdsUpdate update) {
856851
setData(update);
857852
Set<String> addedClusters = Sets.difference(newNames, oldNames);
858853
addedClusters.forEach((cluster) -> addClusterWatcher(cluster, parentContext, depth));
859-
860-
if (addedClusters.isEmpty()) {
861-
maybePublishConfig();
862-
}
863-
} else { // data was set to error status above
864-
maybePublishConfig();
865854
}
866855

867856
} else if (depth <= MAX_CLUSTER_RECURSION_DEPTH) {
868857
setData(update);
869858
update.prioritizedClusterNames()
870859
.forEach(name -> addClusterWatcher(name, parentContext, depth));
871-
maybePublishConfig();
872860
}
873861
break;
874862
default:
875863
Status error = Status.UNAVAILABLE.withDescription(
876864
"aggregate cluster graph exceeds max depth at " + resourceName() + nodeInfo());
877865
setDataAsStatus(error);
878-
maybePublishConfig();
879866
}
867+
maybePublishConfig();
880868
}
881869

882870
public String getEdsServiceName() {

0 commit comments

Comments
 (0)