HoustonPutman commented on code in PR #909:
URL: https://github.com/apache/solr/pull/909#discussion_r921419678
##########
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:
so `constructState` takes in the list of collections that should be
notified. It constructs the whole state, then notifies the watchers of the
"updated" collections afterwards.
> I am not sure if I completely follow the original comments in the code
I do not think that comment still applies... The only times where the return
value of this method is used is in are:
- `forciblyRefreshAllClusterStateSlow()`
- `forceUpdateCollection(String collection)`
- `compareStateVersions(String coll, int version)`
None of these are adding a state watcher. And every creation of a
`StateWatcher` uses `new StateWatcher(collection).refreshAndWatch()`
in `refreshAndWatch()` we find:
```java
DocCollection newState = fetchCollectionState(coll, this);
collectionWatches.updateDocCollection(coll, newState);
synchronized (getUpdateLock()) {
constructState(Collections.singleton(coll));
}
```
Meaning `constructState` is always called on new StateWatchers anyways... So
to end, I think we can remove the `=` in `>=` for that if statement. But we do
need to adhere to the original functionality of the return statement, which is
"has this state changed?"
--
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
