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: [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]
