sabi0 commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505545772
##########
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:
Code here is written safely: it makes a copy of a shared set, processes the
copy and then removes the processed elements from the shared set.
I believe it should be fine.
Unlike, for instance something like this:
https://github.com/sabi0/lucene/blob/0b3f2886eb9715b6d55b4df4f755ef87c8e9a5cc/lucene/replicator/src/java/org/apache/lucene/replicator/nrt/ReplicaNode.java#L695-L698
(Just an example, this code is also fine as all operations on the
'pendingMergeFiles' in that class are inside `synchronized (this)` blocks. But
this is rather fragile. It takes a single mutation outside 'synchronized' to
introduce a hard to find bug.)
--
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
