s1monw commented on a change in pull request #1552: URL: https://github.com/apache/lucene-solr/pull/1552#discussion_r439646861
########## File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java ########## @@ -3257,6 +3320,52 @@ private long prepareCommitInternal() throws IOException { } finally { maybeCloseOnTragicEvent(); } + + if (mergeAwaitLatchRef != null) { + CountDownLatch mergeAwaitLatch = mergeAwaitLatchRef.get(); + // If we found and registered any merges above, within the flushLock, then we want to ensure that they + // complete execution. Note that since we released the lock, other merges may have been scheduled. We will + // block until the merges that we registered complete. As they complete, they will update toCommit to + // replace merged segments with the result of each merge. + config.getIndexWriterEvents().beginMergeOnCommit(); + mergeScheduler.merge(mergeSource, MergeTrigger.COMMIT); + long mergeWaitStart = System.nanoTime(); + int abandonedCount = 0; + long waitTimeMillis = (long) (config.getMaxCommitMergeWaitSeconds() * 1000.0); + try { + if (mergeAwaitLatch.await(waitTimeMillis, TimeUnit.MILLISECONDS) == false) { + synchronized (this) { + // Need to do this in a synchronized block, to make sure none of our commit merges are currently + // executing mergeFinished (since mergeFinished itself is called from within the IndexWriter lock). + // After we clear the value from mergeAwaitLatchRef, the merges we schedule will still execute as + // usual, but when they finish, they won't attempt to update toCommit or modify segment reference + // counts. + mergeAwaitLatchRef.set(null); + for (MergePolicy.OneMerge commitMerge : commitMerges) { + if (runningMerges.contains(commitMerge) || pendingMerges.contains(commitMerge)) { + abandonedCount++; + } + } + } + } + } catch (InterruptedException ie) { + throw new ThreadInterruptedException(ie); + } finally { + if (infoStream.isEnabled("IW")) { + infoStream.message("IW", String.format(Locale.ROOT, "Waited %.1f ms for commit merges", + (System.nanoTime() - mergeWaitStart)/1_000_000.0)); + infoStream.message("IW", "After executing commit merges, had " + toCommit.size() + " segments"); + if (abandonedCount > 0) { + infoStream.message("IW", "Abandoned " + abandonedCount + " commit merges after " + waitTimeMillis + " ms"); + } + } + if (abandonedCount > 0) { + config.getIndexWriterEvents().abandonedMergesOnCommit(abandonedCount); Review comment: I think we should detach this discussion if we need metrics on IW from this PR. It distracts from it's actual core change IMO and if'd add metrics then we'd need some more or can consolidate them too. I'd rather have a stats object than a callback here to be honest but again that's a different discussion. ---------------------------------------------------------------- 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