madrob commented on code in PR #909:
URL: https://github.com/apache/solr/pull/909#discussion_r915426740
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -246,6 +245,125 @@ 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 {
+ private final ConcurrentHashMap<String,
StatefulCollectionWatch<DocCollectionWatcher>>
+ statefulWatchesByCollectionName = new ConcurrentHashMap<>();
+
+ /**
+ * 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 =
+ statefulWatchesByCollectionName.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 statefulWatchesByCollectionName.entrySet().stream()
+ .filter(
+ (Entry<String, StatefulCollectionWatch<DocCollectionWatcher>>
entry) ->
+ entry.getValue().currentState != null)
+ .map(Entry::getKey)
+ .collect(Collectors.toUnmodifiableSet());
+ }
+
+ private Set<String> watchedCollections() {
+ return
Collections.unmodifiableSet(statefulWatchesByCollectionName.keySet());
Review Comment:
Do we really need to wrap this in an unmodifiable set? it's private, we
could javadoc asking callers to not modify this set.
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -246,6 +244,90 @@ 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,
DocCollectionWatch<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) {
+ DocCollectionWatch<DocCollectionWatcher> watch = get(collection);
+ return watch != null ? watch.currentDoc : 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, DocCollectionWatch<DocCollectionWatcher>> entry)
->
+ entry.getValue().currentDoc != null)
+ .map(Entry::getKey)
+ .collect(Collectors.toUnmodifiableSet());
+ }
+
+ /**
+ * Updates the latest observed DocCollection (state) of the {@link
DocCollectionWatch} 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
+ */
+ private boolean updateDocCollection(String collection, DocCollection
newState) {
+ DocCollectionWatch<DocCollectionWatcher> watch = get(collection);
+ if (watch != null) {
+ DocCollection oldState = watch.currentDoc;
+ if (oldState == null && newState == null) {
+ // OK, the collection not yet exist in ZK
+ } else if (oldState == null) {
+ if (log.isDebugEnabled()) {
+ log.debug("Add data for [{}] ver [{}]", collection,
newState.getZNodeVersion());
+ }
+ watch.currentDoc = newState;
+ } else if (newState == null) {
+ log.debug("Removing cached collection state for [{}]", collection);
+ watch.currentDoc = null;
+ } else { // both new and old states are non-null
+ int oldCVersion =
+ oldState.getPerReplicaStates() == null ? -1 :
oldState.getPerReplicaStates().cversion;
+ int newCVersion =
+ newState.getPerReplicaStates() == null ? -1 :
newState.getPerReplicaStates().cversion;
+ if (oldState.getZNodeVersion() < newState.getZNodeVersion()
+ || oldCVersion < newCVersion) {
+ watch.currentDoc = newState;
+ if (log.isDebugEnabled()) {
+ log.debug(
+ "Updating data for [{}] from [{}] to [{}]",
+ collection,
+ oldState.getZNodeVersion(),
+ newState.getZNodeVersion());
+ }
+ }
+ }
+ return true;
+ } else {
+ return false;
+ }
+ }
+ }
+
+ private static class DocCollectionWatch<T> extends CollectionWatch<T> {
Review Comment:
If this is only ever used with `T = DocCollectionWatcher` then we might as
well declare that explicitly and save ourselves typing on the declarations?
There might be other usages that I'm not seeing though. I don't think this is
something that's worth trying to make generic (yet?)
```suggestion
private static class DocCollectionWatch extends
CollectionWatch<DocCollectionWatcher> {
```
--
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
