dsmiley commented on code in PR #2571:
URL: https://github.com/apache/solr/pull/2571#discussion_r1690380168


##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java:
##########
@@ -1228,9 +1199,15 @@ protected DocCollection getDocCollection(String 
collection, Integer expectedVers
       if (col != null) {
         if (expectedVersion <= col.getZNodeVersion() && 
!cacheEntry.shouldRetry()) return col;
       }
+      // Moving the lookup inside the lock makes the lazy/non-lazy check 
irrelevant

Review Comment:
   BTW a comment like "moving the ..." is only pertinent as a code review 
comment but not as a comment in the code.  It's a verb pertaining to a change 
that a commit/PR makes, which generally makes a poor/weird comment (similar to 
what I said in another feedback here today).  I'm not sure I'd bother with this 
comment.   If we are concerned that someone might add a call to 
getCollectionRef above the lock, then we could add a comment up there to warn 
against making remote calls without holding the lock.



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

Reply via email to