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
