mikemccand commented on a change in pull request #1772: URL: https://github.com/apache/lucene-solr/pull/1772#discussion_r475177874
########## File path: lucene/core/src/test/org/apache/lucene/index/TestIndexWriterDelete.java ########## @@ -300,10 +300,11 @@ public void testDeleteAllSimple() throws IOException { modifier.close(); dir.close(); } - + public void testDeleteAllNoDeadLock() throws IOException, InterruptedException { Directory dir = newDirectory(); - final RandomIndexWriter modifier = new RandomIndexWriter(random(), dir); + final RandomIndexWriter modifier = new RandomIndexWriter(random(), dir, + newIndexWriterConfig().setMergePolicy(new MockRandomMergePolicy(random()))); Review comment: Nice -- this forced `merge-on-getReader/commit` to be used more often in this test? ########## File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java ########## @@ -2427,7 +2426,13 @@ public long deleteAll() throws IOException { synchronized (this) { try { // Abort any running merges - abortMerges(); + try { + abortMerges(); + assert merges.areEnabled() == false : "merges should be disabled - who enabled them?"; + assert mergingSegments.isEmpty() : "found merging segments but merges are disabled: " + mergingSegments; + } finally { + merges.enable(); Review comment: Aha, this is the meat of the change! We need to be able to stop all running merges, but not prevent new merges. Maybe add a comment explaining this? The code looks a little odd to `abortMerges` to only then `merges.enable()` in a `finally` clause. The `finally` clause is necessary here because on exception we want to be certain to re-enable merges? An exception during `abortMerges` is not tragic for `IW` right? I.e. it can resume operations, and maybe not all docs were deleted? ---------------------------------------------------------------- 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. 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