aparnasuresh85 commented on code in PR #2585:
URL: https://github.com/apache/solr/pull/2585#discussion_r1691146117
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -0,0 +1,397 @@
+package org.apache.solr.common.cloud;
+
+import static java.util.Collections.emptyMap;
+
+import java.io.Closeable;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkStateReader.CollectionWatch;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.common.util.Utils;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Fetches and manages collection properties from a ZooKeeper ensemble */
+public class CollectionPropertiesZkStateReader implements Closeable {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private volatile boolean closed = false;
+
+ private final SolrZkClient zkClient;
+ private final ZkStateReader zkStateReader;
+
+ /** Collection properties being actively watched */
+ private final ConcurrentHashMap<String, VersionedCollectionProps>
watchedCollectionProps =
+ new ConcurrentHashMap<>();
+
+ /**
+ * Manages ZooKeeper watchers for each collection. These watchers monitor
changes to the
+ * properties of the collection in ZooKeeper. When a change is detected in
ZooKeeper, the watcher
+ * triggers an update, which then notifies the relevant
"collectionPropsObserver".
+ */
+ private final ConcurrentHashMap<String, PropsWatcher>
collectionPropsWatchers =
+ new ConcurrentHashMap<>();
+
+ /**
+ * Manages a list of observers (listeners) for each collection. These
observers need to be
+ * notified when the properties of the collection change. When a
collection's properties change,
+ * all registered observers for that collection are notified by a
"collectionPropWatcher".
+ */
+ private ConcurrentHashMap<String, CollectionWatch<CollectionPropsWatcher>>
+ collectionPropsObservers = new ConcurrentHashMap<>();
+
+ /** Used to submit notifications to Collection Properties watchers in order
*/
+ private final ExecutorService collectionPropsNotifications =
+ ExecutorUtil.newMDCAwareSingleThreadExecutor(
+ new SolrNamedThreadFactory("collectionPropsNotifications"));
+
+ private final ExecutorService notifications =
+ ExecutorUtil.newMDCAwareCachedThreadPool("cachecleaner");
Review Comment:
> I was looking at some of these fields carefully, and I observed that
"notifications" was something you duplicated (like you duplicated "closed"
too).
BTW next time you do a refactor, it's good practice to self code-review in
GitHub to comment on anything non-obvious.
I agree
> Why was this duplicated? In ZkStateReader, this thread pool is named
"watcher" and here you name it "cachecleaner". I see it being used as more than
just the cache cleaner.
It is only being used for cache cleaning purposes within CPZSR
Felt awkward exposing the threadpool from ZSR initially
>In the spirit of refactoring, can we provide access to the notifications
executor to CollectionPropertiesZkStateReader as a kind of general purpose
watching executor? This way no need to shut it down (manage its lifecycle) in
this class.
Done.
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
