dsmiley commented on code in PR #2276:
URL: https://github.com/apache/solr/pull/2276#discussion_r1508136097
##########
solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java:
##########
@@ -630,11 +628,8 @@ public final void doSyncOrReplicateRecovery(SolrCore core)
throws Exception {
.getCollection(cloudDesc.getCollectionName())
.getSlice(cloudDesc.getShardId());
- try {
+ if (prevSendPreRecoveryHttpUriRequest != null) {
Review Comment:
I see this is correct because prevSendPreRecoveryHttpUriRequest never
becomes null once it is non-null. Otherwise a concurrent set to null would
cause an NPE here if timed just right. But this is super subtle! It'd be
clearer, even if slightly more code, to save the field access to a local
variable where you check it then -- a typical pattern when a field can be
accessed concurrently. Perhaps there's a one-liner approach using
`Optional.ofNullable(field).ifPresent(f -> f.cancel(true))`
--
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]
