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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
