patsonluk commented on code in PR #909:
URL: https://github.com/apache/solr/pull/909#discussion_r921636428


##########
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -246,6 +244,95 @@ public boolean canBeRemoved() {
     }
   }
 
+  /**
+   * A ConcurrentHashMap of active watcher by collection name
+   *
+   * <p>Each watcher DocCollectionWatch also contains the latest DocCollection 
(state) observed
+   */
+  private static class DocCollectionWatches
+      extends ConcurrentHashMap<String, 
StatefulCollectionWatch<DocCollectionWatcher>> {
+
+    /**
+     * Gets the DocCollection (state) of the collection which the 
corresponding watch last observed
+     *
+     * @param collection the collection name to get DocCollection on
+     * @return The last observed DocCollection(state). if null, that means 
there's no such
+     *     collection.
+     */
+    private DocCollection getDocCollection(String collection) {
+      StatefulCollectionWatch<DocCollectionWatcher> watch = get(collection);
+      return watch != null ? watch.currentState : null;
+    }
+
+    /**
+     * Gets the active collections (collections that exist) being watched
+     *
+     * @return an immutable set of active collection names
+     */
+    private Set<String> activeCollections() {
+      return this.entrySet().stream()
+          .filter(
+              (Entry<String, StatefulCollectionWatch<DocCollectionWatcher>> 
entry) ->
+                  entry.getValue().currentState != null)
+          .map(Entry::getKey)
+          .collect(Collectors.toUnmodifiableSet());
+    }
+
+    /**
+     * Updates the latest observed DocCollection (state) of the {@link 
StatefulCollectionWatch} if
+     * the collection is being watched
+     *
+     * @param collection the collection name
+     * @param newState the new DocCollection (state) observed
+     * @return whether an active watch exists for such collection

Review Comment:
   Thank you for the detailed explanations @HoustonPutman !
   
   So it appears that `constructState` serves at least 2 purposes:
   1. Construct the clusterstate again as there are changes in the watched 
collection states
   2. Notify all the watchers as defined in `changedCollections` param
   
   So i agree that for 1., it only makes sense to re-construct the state if 
indeed the doc collection has been changed
   
   As for 2, I don't understand enough to tell whether it matters, like whether 
`forciblyRefreshAllClusterStateSlow` would expect all watches be notified, 
whether there's any change on the corresponding collection or not. But i trust 
your analysis!
   
   
    



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