[GitHub] [solr] HoustonPutman commented on a diff in pull request #909: SOLR-16257: `ZkStateReader` changes to avoid race condition between `collectionWatches` and `watchedCollectionStates`

[GitBox <gi...@apache.org>]

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


##########
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:
   Ok it looks like this return signature contract has changed from the previous method.
   
   The previous method (`updateWatchedCollection`) returned whether the state changed. This is returning whether the new state is null. That's quite a difference. Is there a reason you did this? It should be pretty easy to add in an atomic boolean and check whether the state was changed and return that.



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org