dweiss commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505580073
##########
lucene/replicator/src/java/org/apache/lucene/replicator/nrt/PrimaryNode.java:
##########
@@ -158,10 +158,7 @@ public long getPrimaryGen() {
*/
public boolean flushAndRefresh() throws IOException {
message("top: now flushAndRefresh");
- Set<String> completedMergeFiles;
Review Comment:
The code is safe but how it works is different, especially with concurrency
involved. The iteration over a concurrent hash map itself is not blocking
updates to the collection (or other iterators) - often it doesn't matter, but
sometimes it may - depends on the logic outside.
Seeing the patch diff only, it's hard to figure out the scope of such a
change. Even when you see the entire code, it's sometimes tricky to figure out
how it may behave. If this wasn't an automatic replacement and you've reviewed
how those collections are used, I'm fine with the change.
--
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...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
