vigyasharma commented on PR #633: URL: https://github.com/apache/lucene/pull/633#issuecomment-1140767642
> I could either wrap the runningMerges update with a synchronized (IndexWriter.this) {}, or make runningMerges a synchronizedSet. I like the second approach as it automatically fixes this at all other places. I decided to go with the first approach of wrapping critical sections with a `synchronized (IndexWriter.this) {}`. This feels simpler to reason about as we're synchronizing on a single object - `IndexWriter.this`. A `runningMerges` synchronizedSet, would've created three different objects that were getting locked at different places - the IndexWriter object, the synchronizedSet lock, and the AddIndexesMergeSource, which has sync functions so that running and pending merge queues can be transactionally updated. Reasoning about, and avoiding deadlocks is simpler with a single IW lock. And given the nature of these critical sections, I don't think this affects concurrency by much. Also rebased on the latest main. -- 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