clintropolis commented on code in PR #12939:
URL: https://github.com/apache/druid/pull/12939#discussion_r956514980


##########
server/src/main/java/org/apache/druid/curator/inventory/CuratorInventoryManager.java:
##########
@@ -278,7 +278,6 @@ public void childEvent(CuratorFramework client, 
PathChildrenCacheEvent event) th
             // This close() call actually calls shutdownNow() on the executor 
registered with the Cache object, it
             // better have its own executor or ignore shutdownNow() calls...
             log.debug("Closing inventory cache for %s. Also removing 
listeners.", containerKey);
-            removed.getCache().getListenable().clear();

Review Comment:
   i guess this wasn't really necessary to be there in previous version either, 
since it looks like the listeners got cleared there too 
https://github.com/apache/curator/blob/apache-curator-4.3.0/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java#L378



##########
server/src/main/java/org/apache/druid/curator/discovery/DiscoveryModule.java:
##########
@@ -436,6 +430,12 @@ public void start()
         // nothing
       }
 
+      @Override
+      public CountDownLatch startImmediate()
+      {
+        return null;

Review Comment:
   nit: I looked around and don't _think_ anything would be calling this, but 
maybe it would be safer to return a `CountDownLatch` with `0` as the count just 
in case anything does?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to