From 48d08e643eb872647990f6881abe8e5d00b0a307 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Mon, 2 Jun 2025 23:22:38 -0700
Subject: [PATCH] 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.
---
 .../io/grpc/xds/XdsDependencyManager.java     | 20 ++++---------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java b/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java
index 0c3e110119..d786e525c0 100644
--- a/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java
+++ b/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java
@@ -456,17 +456,15 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
     return logId.toString();
   }
 
-  // Returns true if the watcher was added, false if it already exists
-  private boolean addEdsWatcher(String edsServiceName, CdsWatcher parentContext) {
+  private void addEdsWatcher(String edsServiceName, CdsWatcher parentContext) {
     EdsWatcher watcher
         = (EdsWatcher) getWatchers(XdsEndpointResource.getInstance()).get(edsServiceName);
     if (watcher != null) {
       watcher.addParentContext(parentContext); // Is a set, so don't need to check for existence
-      return false;
+      return;
     }
 
     addWatcher(new EdsWatcher(edsServiceName, parentContext));
-    return true;
   }
 
   private void addClusterWatcher(String clusterName, Object parentContext, int depth) {
@@ -823,13 +821,10 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
       switch (update.clusterType()) {
         case EDS:
           setData(update);
-          if (!addEdsWatcher(getEdsServiceName(), this))  {
-            maybePublishConfig();
-          }
+          addEdsWatcher(getEdsServiceName(), this);
           break;
         case LOGICAL_DNS:
           setData(update);
-          maybePublishConfig();
           // no eds needed
           break;
         case AGGREGATE:
@@ -856,27 +851,20 @@ final class XdsDependencyManager implements XdsConfig.XdsClusterSubscriptionRegi
               setData(update);
               Set<String> addedClusters = Sets.difference(newNames, oldNames);
               addedClusters.forEach((cluster) -> addClusterWatcher(cluster, parentContext, depth));
-
-              if (addedClusters.isEmpty()) {
-                maybePublishConfig();
-              }
-            } else { // data was set to error status above
-              maybePublishConfig();
             }
 
           } else if (depth <= MAX_CLUSTER_RECURSION_DEPTH) {
             setData(update);
             update.prioritizedClusterNames()
                 .forEach(name -> addClusterWatcher(name, parentContext, depth));
-            maybePublishConfig();
           }
           break;
         default:
           Status error = Status.UNAVAILABLE.withDescription(
               "aggregate cluster graph exceeds max depth at " + resourceName() + nodeInfo());
           setDataAsStatus(error);
-          maybePublishConfig();
       }
+      maybePublishConfig();
     }
 
     public String getEdsServiceName() {