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 ..." (and referring to a check that no longer 
exists) 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