kgyrtkirk commented on code in PR #17482: URL: https://github.com/apache/druid/pull/17482#discussion_r2039306679
########## docs/configuration/index.md: ########## @@ -148,6 +148,7 @@ We recommend just setting the base ZK path and the ZK service host, but all ZK p |`druid.zk.service.connectionTimeoutMs`|ZooKeeper connection timeout, in milliseconds.|`15000`| |`druid.zk.service.compress`|Boolean flag for whether or not created Znodes should be compressed.|`true`| |`druid.zk.service.acl`|Boolean flag for whether or not to enable ACL security for ZooKeeper. If ACL is enabled, zNode creators will have all permissions.|`false`| +|`druid.zk.service.pathChildrenCacheStrategy`|Dictates the underlying caching strategy for service announcements. Set true to let announcers to use Apache Curator's PathChildrenCache strategy, otherwise NodeCache strategy. Consider using NodeCache strategy when you are dealing with huge number of ZooKeeper watches in your cluster.|`true`| Review Comment: I wonder why not make the `NodeCache` approach the default as that should work better - but retain the old approach if some issue happens? cc: @cryptoe ########## 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: `NodeAnnouncer` and `PathChildrenAnnouncer` share a lot of common pieces ; I wonder if it could be placed into a common abstract - or the old approach should be removed after the cache is proven to work correctly? -- 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]
