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

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

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


##########
solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -326,12 +408,11 @@ public void forciblyRefreshAllClusterStateSlow() throws KeeperException, Interru
       // No need to set watchers because we should already have watchers registered for everything.
       refreshCollectionList(null);
       refreshLiveNodes(null);
-      // Need a copy so we don't delete from what we're iterating over.
-      Collection<String> safeCopy = new ArrayList<>(watchedCollectionStates.keySet());
+
       Set<String> updatedCollections = new HashSet<>();
-      for (String coll : safeCopy) {
+      for (String coll : collectionWatches.keySet()) {

Review Comment:
   Ah yes. This is one of the change that Im not 100% sure as it changes the behavior. It was buried in one of the original questions:
   
   1. In `forciblyRefreshAllClusterStateSlow`, which we are proposing to just replace `watchedCollectionStates.keySet()` with `collectionWatches.keySet()`, the reasoning is that we should forcibly update every collection that is registered in `collectionWatches`, even if previous fetch on such collection returned null (ie `watchedCollectionStates` would have no entry on it). This is a minor change of behavior but I think it is more "correct"? Would really need more experienced Solr dev to share their thoughts here 🙏🏼 



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