Merges on commit and getReader never call the merge policy's OneMerge#mergeInit

Since 8.6.0, commit() and, since 8.7.0, getReader can merge small segments before they return, using the merges the policy returns from findFullFlushMerges; since 9.3.0 this is on by default with a 500 ms wait. For these merges IndexWriter wraps each policy OneMerge in an anonymous OneMerge (IndexWriter.preparePointInTimeMerge) and runs only the wrapper. The wrapper forwards mergeFinished, onMergeComplete, wrapForMerge, reorder and setMergeInfo to the policy's merge, but has no mergeInit override. So a policy's OneMerge that overrides mergeInit gets mergeFinished with no mergeInit before it on every such merge: mergeFinished(true, false) for a merge that ran and was committed, as in the repro; the same class from findMerges or forceMerge gets both. A policy does not have to override findFullFlushMerges: the default returns its own findMerges merges below maxFullFlushMergeSize(), a limit that LogMergePolicy and TieredMergePolicy set (the base MergePolicy returns 0). Stock Lucene shows no effect, because the skipped default only records the merge thread and nothing in Lucene overrides mergeInit. Main and every release from 8.6.0 to 10.6.0 fail the repro (ported for 8.6.0-9.0.0, 9.1 and 9.2); a 6-line forward fixes it. Low priority.

• OneMerge hooks: MergePolicy.OneMerge is public (@lucene.experimental). mergeInit() is "Called by IndexWriter after the merge started and from the thread that will be executing the merge"; mergeFinished(success, segmentDropped) is "Called by IndexWriter after the merge is done and all readers have been closed" (MergePolicy.java:268-282).
• Merge on commit / getReader ("full-flush merges"): findFullFlushMerges (MergePolicy.java:664-714) and IndexWriterConfig#setMaxFullFlushMergeWaitMillis, default DEFAULT_MAX_FULL_FLUSH_MERGE_WAIT_MILLIS = 500 (IndexWriterConfig.java:108).

1. commit() calls preparePointInTimeMerge(..., MergeTrigger.COMMIT, ...) when the wait is above 0 (IndexWriter.java:3858-3864); getReader calls it with GET_READER (:634-638).
2. preparePointInTimeMerge (:3950-4110) runs the configured policy through a OneMergeWrappingMergePolicy whose function replaces every merge with new MergePolicy.OneMerge(toWrap) { ... } (:3959-3964). IndexWriter registers and runs only that wrapper.
3. The wrapper forwards toWrap.mergeFinished (:4027), toWrap.onMergeComplete (:4043), toWrap.wrapForMerge and toWrap.reorder (both commented // must delegate, :4061, :4067) and toWrap.setMergeInfo (:4073). It has no mergeInit override.
4. IndexWriter.merge -> mergeInit(merge) (:5075) -> _mergeInit -> merge.mergeInit() on the wrapper (:5289). The base method only records the merge thread in the wrapper's own progress object (MergePolicy.java:272-274; the wrapping constructor creates a fresh one, :259-266).
5. When the merge closes, OneMerge.close calls the wrapper's mergeFinished (MergePolicy.java:285-293), which passes its arguments on as toWrap.mergeFinished(committed, segmentDropped) (:4027); for the repro's merge that is (true, false).

Two commits of one empty document each, with a policy whose findFullFlushMerges returns one counting OneMerge over both segments. After the second commit the commit point has 1 segment and the policy's OneMerge has mergeFinished = 1 and mergeInit = 0. The same counting class handed out by findMerges (an explicit maybeMerge()) or findForcedMerges gets 1 and 1.

• A merge on DirectoryReader.open(writer) (GET_READER), and a merge on commit with the default ConcurrentMergeScheduler: events [finished:true:false] instead of [init, finished:true:false], 10 of 10 iterations each.
• A policy that overrides only findMerges on a stock LogDocMergePolicy, default settings: the default findFullFlushMerges returns the same objects, which get init=0 finished=1. (The default keeps merges whose segments are below maxFullFlushMergeSize(): minMergeSize in LogMergePolicy, floorSegmentBytes in TieredMergePolicy, 0 in the base MergePolicy, MergePolicy.java:730-736.)

IndexWriter does not always pair the two hooks: on rollback, merges that never started get mergeFinished(false, false) without mergeInit (IndexWriter.java:2821-2834, :5133-5136, :5394-5403), which the javadoc allows. Only the success case on this path is the bug. The repro's merge succeeds and keeps its segment, so it gets mergeFinished(true, false); a merge that fails or whose segment is dropped would get success=false or segmentDropped=true, also without mergeInit (code reading). An application can create the same asymmetry on any path with its own code, by installing a OneMergeWrappingMergePolicy whose wrapper forwards mergeFinished but not mergeInit; IndexWriter's other paths run the policy's own merge. By code reading only (untested): because the wrapper has its own progress object, the policy's OneMerge also never sees isAborted() == true, and IndexWriter sets the merge byte counts on the wrapper.

• Who is exposed. Applications or libraries whose merge policy hands out a OneMerge subclass that overrides mergeInit, for merges that run inside commit(), prepareCommit() or getReader / DirectoryReader.open(writer). On by default since 9.3.0; opt-in (a full-flush wait above 0) in 8.6.0-9.2.x.
• What they see. mergeFinished with no mergeInit before it, on every such merge; for a merge that succeeds and keeps its segment, as in the repro, that is mergeFinished(true, false). Anything paired across the two hooks gets out of step: a timer never started, a per-merge resource released without being taken, a running-merge counter that goes negative. What follows is defined by that code, not by Lucene.
• Scale. None needed: two one-document segments, every time, assertions on and off.
• Stock Lucene. No observable effect. The skipped base method only writes the private field OneMergeProgress.owner, "for sanity-check purposes only" (MergePolicy.java:106-111), which no code reads except an assert in its own setter, and no class in Lucene overrides mergeInit (at 645d4fe14a20). Merged content, deletes, search results, aborting the running merge and the commit wait bound are unaffected (code reading).
• How likely. Rare: GitHub code searches found no mergeInit override in Elasticsearch, OpenSearch or Solr (not exhaustive), and nobody has reported it since 8.6.0.

Public API only (FilterMergePolicy, LogDocMergePolicy, the public OneMerge(List) constructor and hooks, MergeSpecification, SerialMergeScheduler, SegmentInfos.readLatestCommit, ByteBuffersDirectory; the test framework's LuceneTestCase). It sits in org.apache.lucene.index next to the core tests but uses no package-private API. The first two assertions (1 segment, mergeFinished once) pass on stock code and prove that the merge on commit ran and was committed; the third fails. With SerialMergeScheduler, a non-zero wait always waits for the full-flush merge to finish, whatever its value (IndexWriterConfig.java:495-497): that makes the test deterministic, and any wait above 0 turns the path on. The tie-break ran the same path with the default ConcurrentMergeScheduler (wait raised to 60 s): same result, 10 of 10.

The same test is the suggested regression test. Before 9.3.0 merge on commit is off by default (wait 0): on 9.1.x and 9.2.x add setMaxFullFlushMergeWaitMillis(500); on 8.6.0-9.0.0 also use the old test-framework package (org.apache.lucene.util.LuceneTestCase), and on 8.6.x the setter is setMaxCommitMergeWaitMillis (from 8.7.0 on it is setMaxFullFlushMergeWaitMillis).

# gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (environment lines cut)
TestMergeOnCommitCallsMergeInit > testMergeOnCommitCallsMergeInit FAILED
    java.lang.AssertionError: mergeInit calls before mergeFinished expected:<1> but was:<0>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:24681E7CA4B8BF07]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.apache.lucene.index.TestMergeOnCommitCallsMergeInit.testMergeOnCommitCallsMergeInit(TestMergeOnCommitCallsMergeInit.java:67)

1 test completed, 1 failed

# -Ptests.asserts=false: the same failure and message
# with the fix: 1 test, 0 failures (assertions on and off)

• Contract. mergeInit is public and non-final, documented as "Called by IndexWriter after the merge started and from the thread that will be executing the merge" (MergePolicy.java:268-271). No javadoc, CHANGES or MIGRATE entry exempts full-flush merges.
• Meant to be used. LUCENE-7700 refactored merge throughput control "into a more accessible set of public methods (see MergePolicy.OneMergeProgress, MergeScheduler.wrapForMerge and OneMerge.mergeInit)" (lucene/CHANGES.txt:8405-8409).
• The wrapper treats the policy's merge as the real one. It forwards mergeFinished, onMergeComplete, setMergeInfo, wrapForMerge and reorder (IndexWriter.java:4027-4073). IndexWriterConfig.java:482-487 says these merges "will still run to completion independent of the commit or getReader call, like natural segment merges".
• Precedent on this wrapper. #12622 (9.9.0) added the reorder and setMergeInfo forwards. Asked in review "did we have a test that realized that merge.info is missing?", the author answered "Yes, some of the new tests failed ... Let me add dedicated tests for setMergeInfo."; TestIndexWriterMergePolicy.testSetDiagnostics wraps both findMerges and findFullFlushMerges merges.
• No documented rationale found. The wrapper never had mergeInit (first version 7f352a96659, #1617); no commit in the history contains toWrap.mergeInit; the design threads that were read never discuss it. That is an absence of evidence, not proof that the omission was unintended.

The strongest case, built by verifiers and the adjudication: IndexWriter executes the wrapper and does call mergeInit on it, which satisfies "called after the merge started"; the base method touches only the wrapper's own progress object. Other in-tree wrappers forward even less (SoftDeletesRetentionMergePolicy only wrapForMerge, BPReorderingMergePolicy neither mergeInit nor mergeFinished), and open PR #16571 adds a FilterOneMerge that forwards neither, with the note "Wrapping creates a new OneMerge, so any behaviour not forwarded is lost." IndexWriter already sends mergeFinished(false, false) without mergeInit on rollback, OneMerge is experimental, and nobody overrides mergeInit.

Why it does not settle it: those wrappers forward no lifecycle hook at all, so their inner merge sees a consistent, empty lifecycle. This wrapper forwards mergeFinished, onMergeComplete and setMergeInfo, so the policy's merge hears that it finished successfully without hearing that it started, which happens on no other IndexWriter path (an application gets it elsewhere only by installing such a wrapper itself). The user never asked for this wrapper: IndexWriter puts it around every merge the policy returns from findFullFlushMerges, so avoiding it means returning no full-flush merges or turning merge on commit off. The rollback case is a merge that never started, which the javadoc allows. The javadoc does not say whether the hooks reach a merge that IndexWriter wraps, so a maintainer can reasonably prefer to document mergeInit as plumbing of the executing merge instead of forwarding it; that remedy question is what remains open. Verdict (adjudication): confirmed, medium.

Option A was run through gradle in a scratch worktree of 645d4fe14a20. B, C and D are alternatives that were not run.

Add a mergeInit override to the anonymous OneMerge in preparePointInTimeMerge that calls the wrapper's own base method and then the policy's, next to the existing // must delegate forwards.

@Override
public void mergeInit() throws IOException {
  super.mergeInit();
  toWrap.mergeInit(); // must delegate
}

Trade-offs

• The policy's mergeInit then runs exactly where it runs for natural merges: inside _mergeInit, under IndexWriter's lock, on the merge thread. If it throws, IndexWriter handles it as for a natural merge: mergeInit(merge) runs its internal mergeFinish(merge) bookkeeping (not OneMerge#mergeFinished) and rethrows (IndexWriter.java:5261-5269); what the policy's merge sees after its own mergeInit throws was not tested.
• With assertions on, setMergeThread's assert this.owner == null holds for the policy's merge, which has its own progress object.
• It does not share the two progress objects, so the untested isAborted point stays.
• Open PR #16571 rewrites the same wrapper on top of a new FilterOneMerge; the same override fits there.

What the checks validated

• Gradle in a scratch worktree of 645d4fe14a20 (--offline, seed DEADBEEF). Stock: the repro fails with assertions on and off.
• With the fix: the repro plus TestIndexWriterMergePolicy, TestIndexWriterReader, TestIndexWriter, TestTieredMergePolicy, TestLogMergePolicy, TestOneMergeWrappingMergePolicy, TestFilterMergePolicy, TestConcurrentMergeScheduler, TestDemoParallelLeafReader, TestTragicIndexWriterDeadlock, TestSoftDeletesRetentionMergePolicy and TestIndexWriterExceptions pass with assertions on and off (13 suites, 280 tests, 0 failures, 10 skipped).
• The whole org.apache.lucene.index test package with the fix (215 suites, 1396 tests, 0 failures, 56 skipped), and lucene/misc TestBPReorderingMergePolicy (3 tests, 0 failures, 2 skipped).
• Lint with the fix and the repro in place: checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest pass.
• Earlier: the same forward passed the tie-break's getReader and ConcurrentMergeScheduler variants (5 of 5, -ea and -da) and, patched into 8.6.0's IndexWriter, the 8.6.0 port.

Say in OneMerge#mergeInit and findFullFlushMerges that for merges on commit and getReader IndexWriter runs its own wrapper and forwards only mergeFinished, onMergeComplete, setMergeInfo, wrapForMerge and reorder.

Trade-offs

No behaviour change and cheap, but a successful mergeFinished without mergeInit stays on one path, so every policy author has to handle it.

Let the wrapper use the policy merge's OneMergeProgress (a new protected constructor), so abort and pause state are shared as well.

Trade-offs

Also covers the untested isAborted point, but changes the experimental wrapping constructor's semantics; a larger review.

Only if that PR lands: add mergeInit (and mergeFinished) to FilterOneMerge.

Trade-offs

Changes every wrapper built on FilterOneMerge, including ones that today forward no lifecycle hook; option A on the rewritten wrapper is narrower.

Method: a bytecode delegation scan, then JUnit (the hunt's "semi-formal" tier).

• A hunt unit (merge-policy hooks) extended an earlier unit's delegation-completeness scanner (MergeHookScan.java, JDK 25 java.lang.classfile) to every OneMerge and MergePolicy wrapper and every overridable hook: 6076 classes from 20 module jars, 11 wrappers, 140 matrix rows.
• It resolves each hook on each wrapper with JVM method selection and records whether the wrapper forwards it to the wrapped object. The gap row IndexWriter$2 mergeInit()V lifecycle/result BASE (the anonymous OneMerge in preparePointInTimeMerge) said that mergeInit resolves to the base default while five sibling hooks forward.
• Vacuity check: a planted wrapper that drops only mergeInit is flagged and a complete one is not.
• An exploratory probe and then a public-API JUnit test on the stock jars (merge on commit, merge on getReader, a forceMerge control) confirmed it on main (assertions on and off), 10.5.2 and 9.12.3.