GWphua commented on code in PR #17482:
URL: https://github.com/apache/druid/pull/17482#discussion_r2051668845


##########
server/src/main/java/org/apache/druid/curator/announcement/PathChildrenAnnouncer.java:
##########
@@ -102,11 +112,13 @@ Set<String> getAddedChildren()
   }
 
   @LifecycleStart
+  @Override
   public void start()

Review Comment:
   Hi @kgyrtkirk, I have taken a look at making an abstract -- It is true that 
there's alot of shared methods, but it is not trivial to make the changes. I am 
considering this instead
   
   1. We give some time to use the NodeAnnouncer instead, if it proves to 
perform better, we can simply delete the PathChildrenCacheAnnouncer.
   2. If using NodeCache provides a trade-off of CPU for Memory, a PR can 
further work towards replacing the deprecated PathChildrenAnnouncer using 
CuratorCache. We can decide to make an Abstract class then, as some of the 
complexities (such as `listeners` having type of `ConcurrentMap<String, 
PathChildrenCache>` will be changed to a common `ConcurrentMap<String, 
CuratorCache>`). This seems to be the direction where Apache Curator is trying 
to go with their caches.



-- 
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