[GitHub] [lucene-solr] msfroh commented on a change in pull request #1552: LUCENE-8962: merge small segments on commit

[GitBox <gi...@apache.org>]

msfroh commented on a change in pull request #1552:
URL: https://github.com/apache/lucene-solr/pull/1552#discussion_r442043040



##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3255,7 +3302,16 @@ private long prepareCommitInternal() throws IOException {
       } finally {
         maybeCloseOnTragicEvent();
       }
-     
+
+      if (onCommitMerges != null) {
+        mergeScheduler.merge(mergeSource, MergeTrigger.COMMIT);

Review comment:
       Would it be sufficient to document the behavior in the Javadoc for `findFullFlushMerges`?
   
   I was assuming that any implementation of `findFullFlushMerges` would try to return merges that are very likely complete within whatever timeout someone would reasonably set (e.g. a few seconds). The timeout was intended just as an extra safeguard in case a merge takes longer. 
   
   Given that lots of IndexWriter operations can have pauses with `SerialMergeScheduler` (judging by the number of calls to `maybeMerge`, especially the one from `processEvents`, in IndexWriter), blocking on this particular `merge` call doesn't feel like it introduces more risk (especially since it needs to be used in conjunction with a `MergePolicy` that implements `findFullFlushMerges`).




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org