magibney commented on code in PR #1964:
URL: https://github.com/apache/solr/pull/1964#discussion_r1341703136
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -1329,8 +1336,9 @@ public void process(WatchedEvent event) {
return;
}
- if (!collectionWatches.watchedCollections().contains(coll)) {
- // This collection is no longer interesting, stop watching.
+ StatefulCollectionWatch scw =
collectionWatches.statefulWatchesByCollectionName.get(coll);
+ if (scw == null || scw.associatedWatcher != this) {
+ // Collection no longer interesting, or we have been replaced by a
different watcher.
Review Comment:
The substance of the fix is in these three lines. Since StateWatcher
instance do a deferred check against collectionWatches in order to determine
whether to exit, it's possible that by the time a StateWatcher checks here, the
watch it was created to serve has been removed, and a new watch inserted (and
new StateWatcher created) before this check. In that case it's possible to
accumulate persistent StateWatcher instances that all notify callbacks on the
same event.
The fix is for the StateWatcher to check whether it is the instance
responsible for the current entry in collectionWatches, and exit if not. The
rest of the changes are basically just to ensure that The new
StatefulCollectionWatch.associatedWatcher field is never null once it's been
inserted in the map.
--
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]
