LogMergePolicy's forceMergeDeletes leaves deletes in idle segments that share a run with an already-merging segment

With LogDocMergePolicy or LogByteSizeMergePolicy, IndexWriter.forceMergeDeletes(true) called while another merge holds a segment in the same run of adjacent segments with deletes can return with deleted documents still stored in segments that were never part of any merge. LogMergePolicy.findForcedDeletesMerges groups adjacent segments with deletes into runs without looking at the segments that are already merging; IndexWriter.registerMerge rejects any run that contains such a segment and reports it only through the infoStream, when that is enabled; and forceMergeDeletes asks the policy once and waits only for merges that were registered. In the minimal test the idle 10-document segment keeps its 1 deleted document; in a larger, hook-free scenario a 5,000-document segment keeps all 500 deletes. Deleted documents stay invisible to searches and nothing is corrupted. The default TieredMergePolicy skips merging segments and is not affected by this mechanism. The behaviour dates from the API's creation (2.4.0) and is present in every release up to 10.6.0 and on main.

1. IndexWriter.forceMergeDeletes(boolean doWait) (IndexWriter.java:2345-2406) first flushes (:2348). A flush that writes a segment or applies deletes calls maybeMerge(FULL_FLUSH) (:4378-4379), which can register a natural merge; with ConcurrentMergeScheduler it then runs in a background thread.
2. Under the writer lock it calls findForcedDeletesMerges once and passes each proposed merge to registerMerge, ignoring the boolean result (:2358-2364).
3. LogMergePolicy.findForcedDeletesMerges (LogMergePolicy.java:404-465) walks the segments in order. A segment with numDeletesToMerge > 0 starts or extends a run; a run is emitted at mergeFactor segments, at the next segment without deletes, or at the end. It never reads mergeContext.getMergingSegments(), so a run can contain a segment that is already pending or running.
4. IndexWriter.registerMerge (IndexWriter.java:5158-5183) finds that segment in mergingSegments, writes reject merge ... is already marked for merge to the infoStream if the IW component is enabled (:5173-5181) and returns false. The whole merge is dropped, idle segments included.
5. The wait loop (IndexWriter.java:2384-2397) waits only while one of the proposed merges is in pendingMerges or runningMerges. A rejected merge is in neither, so the call returns at once; with ConcurrentMergeScheduler it can return while the conflicting merge is still running. There is no second policy call and no cascade: findForcedDeletesMerges is called only at :2359, and when a merge finishes IndexWriter asks the policy only for natural merges (updatePendingMerges(..., MERGE_FINISHED, ...), :5105, which calls findMerges, :2508).

LogDocMergePolicy, mergeFactor 2. Segments _0 (10 docs, 1 deleted), _1 (2 docs, 1 deleted) and _2 (1 doc). Flushing _2 registers the natural merge {_1, _2}, held pending by the test's scheduler. forceMergeDeletes(true): the policy logs add merge 0 to 1 inclusive, IndexWriter logs reject merge _0(11.0.0):c10/1 _1(11.0.0):c2/1: segment _1(11.0.0):c2/1 is already marked for merge, the natural merge runs, and the index ends as _0(11.0.0):c10/1 _3(11.0.0):c2: 1 deleted document left instead of 0.

One caller thread, stock ConcurrentMergeScheduler, LogDocMergePolicy(mergeFactor=3): S1 = 5,000 docs with 500 deleted, S2 and S3 = 20 docs with 2 deleted each, plus 20 buffered docs. The call's own flush writes S4 (no deletes) and starts the natural merge S2+S3+S4. The policy proposes one merge, [S1, S2, S3], which IndexWriter rejects because S2 is already marked for merge; after all merges finish the index is _0(11.0.0):c5000/500 _4(11.0.0):c56. It failed 50 of 50 runs with assertions on and 19 of 20 with assertions off on main ea57360912ed, and 10 of 10 on 10.5.2 and on 9.12.3.

TieredMergePolicy.findForcedDeletesMerges drops merging segments before it groups (TieredMergePolicy.java:938, :947, :960), so it is not affected by this mechanism (it still leaves segments below its forceMergeDeletesPctAllowed threshold, 10% by default, on purpose), and so does TemporalMergePolicy (TemporalMergePolicy.java:540-545). LogMergePolicy's own natural-merge path (findMerges) has checked merging segments since LUCENE-2904 (LogMergePolicy.java:497, :604, :638). forceMerge copes with conflicts by taking over pending and running merges and cascading (IndexWriter.java:2260-2278, :5020-5025).

LFV-207 has the same trigger and a different defect: the MergeObserver that forceMergeDeletes returns also tracks the rejected merges, so with doWait=false it never completes. Its fix is in IndexWriter; that fix alone does not make this repro pass.

• Who is exposed. Applications that configure LogDocMergePolicy or LogByteSizeMergePolicy (or a subclass) and call forceMergeDeletes while a merge holds a segment in the same run of neighbouring segments with deletes. The default merge policy is TieredMergePolicy (LiveIndexWriterConfig.java:143) since 3.2; from 2.4.0 to 3.1 LogByteSizeMergePolicy was the default, so the default configuration was affected then (by reading).
• Reachable with the stock scheduler once opted in, but timing-dependent. The default scheduler is ConcurrentMergeScheduler (LiveIndexWriterConfig.java:137), and the conflicting merge can be the one the call's own flush starts, so even waiting for merges before the call does not rule it out. It needs a merging segment with deletes in the same run as an idle segment with deletes; the hook-free rates (50 of 50, 19 of 20) are from one setup. IndexWriter.waitForMerges() is package-private (IndexWriter.java:2878), and no javadoc asks callers to wait first.
• What it costs. Deleted documents stay stored in segments that were never merging, for example 500 of 5,000 documents (10% of that segment), using disk space and search work until a second call or some later merge removes them; a later natural merge may pick the segment up, but nothing makes it do so soon.
• Silent. The blocking call returns normally; if the IW infoStream is enabled it logs the rejection, otherwise nothing reports it. hasDeletions() still returns true.
• Not affected. Search results (an executed probe found 0 hits for 506 deleted ids), index integrity (CheckIndex clean), data (nothing lost).
• Workaround. Call forceMergeDeletes(true) again once the conflicting merge has finished; on an idle writer a second call reclaimed everything. On a LogMergePolicy index hasDeletions() tells whether deletes remain (with TieredMergePolicy deletes below its threshold stay on purpose, so it is no success test there). With ConcurrentMergeScheduler, its public sync() waits only for merge threads that are already running (not run as a workaround here).

Every API the test uses is public; it sits in org.apache.lucene.index only to live next to the other merge-policy tests. LuceneTestCase randomizes the codec, so the command fixes the seed; a second seed (C0FFEE) fails the same way. Segments are written with flush(), not commit(): with commit() the default MergePolicy.findFullFlushMerges would register the {b, c} merge as a merge-on-commit and the commit would wait for it.

The same test is the suggested regression test: it fails today and passes with the recommended fix. Lucene's build checks (google-java-format, ECJ, forbidden APIs) pass with it in place.

# main 645d4fe14a20, gradle, -Ptests.seed=DEADBEEF -Ptests.asserts=true (and false)
TestLogMergePolicyForceMergeDeletesWhileMerging > testForceMergeDeletesWhileNaturalMergeIsPending FAILED
    java.lang.AssertionError: deleted docs after forceMergeDeletes(true) expected:<0> but was:<1>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:B4BBB28F682B14EB]:0)

# infoStream of the same scenario
IW: add merge to pendingMerges: _1(11.0.0):c2/1 _2(11.0.0):c1 [total 1 pending]
IW: forceMergeDeletes: index now _0(11.0.0):c10/1 _1(11.0.0):c2/1 _2(11.0.0):c1
MP:   add merge 0 to 1 inclusive
IW: reject merge _0(11.0.0):c10/1 _1(11.0.0):c2/1: segment _1(11.0.0):c2/1 is already marked for merge
IW: merge seg=_3 _1(11.0.0):c2/1 _2(11.0.0):c1
    (index at close: _0(11.0.0):c10/1 _3(11.0.0):c2)

# 10.6.0 and 10.5.2, assertions on and off: the same AssertionError
# with the recommended fix: BUILD SUCCESSFUL, the test passes in both modes

Contract.

• LogMergePolicy.java:404-407: "Finds merges necessary to force-merge all deletes from the index. We simply merge adjacent segments that have deletes, up to mergeFactor at a time."
• MergePolicy.java:654-656: "Determine what set of merge operations is necessary in order to expunge all deletes from the index."
• IndexWriter.java:2408-2411: "Forces merging of all segments that have deleted documents. The actual merges to be executed are determined by the MergePolicy. For example, the default TieredMergePolicy will only pick a segment if the percentage of deleted docs is over 10%." The only exception it names is TieredMergePolicy's threshold.
• lucene/CHANGES.txt:19981-19984 (2.4.0, LUCENE-325): "Added IndexWriter.expungeDeletes methods, which consult the MergePolicy to find merges necessary to merge away all deletes from the index."

Tests. TestIndexWriterMerging.testForceMergeDeletes, testForceMergeDeletes2 and testForceMergeDeletes3 (with newLogMergePolicy, maxDoc == numDocs afterwards) and TestIndexWriterReader.testForceMergeDeletes (assertFalse(r.hasDeletions())) assert that no deletes remain with LogMergePolicy, on an idle writer. No upstream test checks the result of forceMergeDeletes on LogMergePolicy while a merge is pending or running (RandomIndexWriter may call forceMergeDeletes() at random, with a randomized policy and scheduler, but asserts nothing about deletes).

Intent. IndexWriter.getMergingSegments is documented as "to be used by a MergePolicy to avoid selecting merges for segments already being merged" (IndexWriter.java:2520-2523). LUCENE-2904 (#3978, 3.2) added that check to LogMergePolicy's natural merges only. In LUCENE-3569 (#4643), Michael McCandless wrote in 2011 that a forced-deletes selection "would have to check which merges are already running/registered (IW.getMergingSegments) and "work around" them. EG, if there are 7 segments with deletions, you check and see that 4 of them are already merging / scheduled for merge, so you know you only have to merge the other 3."

The strongest case for "intended": IndexWriter rejects conflicting merges by design (in the LUCENE-847 merge-policy design discussion, 2007, Michael McCandless agreed to keep the check for merges already in progress in IndexWriter: "OK, let's leave eligibility check in IW"), and forceMergeDeletes hands the choice to the policy, so leaving deletes behind while other merges run is best effort; callers who need every delete gone should wait for merges first. Maintainers may also prefer an IndexWriter-side retry or a javadoc note to a policy change.

Why it fails: the policy did choose segment a, and IndexWriter then discarded that choice; LogMergePolicy has no threshold that would excuse a. Rejection is harmless for natural merges because the policy is asked again at the next flush or merge, but forceMergeDeletes asks once and has no cascade, unlike forceMerge. No javadoc says to wait for merges first, IndexWriter.waitForMerges() is package-private, and the call's own flush can start the conflicting merge after any such wait. The in-core policies that choose forced-deletes merges themselves (TieredMergePolicy, TemporalMergePolicy) skip merging segments; the other core implementations return null or delegate. No javadoc, CHANGES entry, test, issue, PR or list thread calls this best effort. Verdict: a defect, not intended.

The LogMergePolicy change was built and checked in a scratch worktree of 645d4fe14a20; the IndexWriter retry was not built.

Workaround for users today: call forceMergeDeletes(true) again once the conflicting merge has finished (on an idle writer a second call reclaimed everything); on a LogMergePolicy index hasDeletions() tells whether deletes remain. IndexWriter.waitForMerges() is not public; with ConcurrentMergeScheduler, its public sync() waits for running merge threads (not run as a workaround here).

In findForcedDeletesMerges (+4/-1), read mergeContext.getMergingSegments() once and treat a merging segment like a segment without deletes. The run before it is then emitted through the existing "end of a sequence" branch (LogMergePolicy.java:441-451), and a new run can start after it. In the minimal test the policy then proposes {a} alone.

     assert mergeContext != null;
+    final Set<SegmentCommitInfo> mergingSegments = mergeContext.getMergingSegments();
     for (int i = 0; i < numSegments; i++) {
       final SegmentCommitInfo info = segmentInfos.info(i);
       int delCount = mergeContext.numDeletesToMerge(info);
       assert assertDelCount(delCount, info);
-      if (delCount > 0) {
+      // a segment that is already being merged ends the current run: IndexWriter would reject a
+      // merge that includes it (and its deletes are reclaimed by the running merge anyway)
+      if (delCount > 0 && mergingSegments.contains(info) == false) {

Trade-offs

• No change when nothing is merging (the set is empty), so idle-writer behaviour and the existing tests are unchanged.
• Runs can be split into smaller merges (even one segment) while a merge is in flight.
• The merging segment's deletes are left to the merge that holds it; deletes applied to it after that merge started are carried into the merged segment, as before (older behaviour, not claimed here).
• getMergingSegments() is read under the writer lock, which forceMergeDeletes holds around both the policy call and registerMerge (IndexWriter.java:2358-2364), so the set cannot change in between.
• Fixes LogMergePolicy only: a custom policy that makes the same mistake still meets the IndexWriter half.

What the checks validated

• The minimal test fails without the fix and passes with it, through gradle, assertions on and off.
• With the fix: TestLogMergePolicy, TestIndexWriterMerging, TestIndexWriterMergePolicy and the new test, 48 tests (4 skipped: 2 nightly, 2 @AwaitsFix for issue #14483), 0 failures in both modes; every core org.apache.lucene.index.*Merg* suite plus TestIndexWriterReader and TestIndexWriterDelete, 52 suites, 272 tests (15 skipped), 0 failures; checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest pass.
• Earlier, by the hunt: the independent reproducer's 5 tests and the hook-free probe (50 of 50) pass with the same change; the bounded-exhaustive check found 20,384 violations in 100,800 checks per LogMergePolicy without the change and 0 violations with it (109,952 checks for LogByteSizeMergePolicy in that run); the change applied to 3.6.2 passes the old-line port.

When registerMerge returns false for a merge proposed by findForcedDeletesMerges, wait for the conflicting merges and ask the policy again, or carry the request over the way forceMerge does (IndexWriter.java:2260-2278, :5020-5025). Not built.

Trade-offs

Also covers custom policies, but changes IndexWriter for every policy, can make the blocking call wait for long unrelated natural merges, and needs a guard against re-asking forever when a policy keeps proposing the same conflicting merge. It touches the same lines as the LFV-207 fix (keep only registered merges in the returned MergeObserver).

The LogMergePolicy change for its own correctness, the IndexWriter retry for robustness against any policy.

Trade-offs

Most code and most review.

Method: semi-formal (bounded-exhaustive translation validation of the real code).

• Check. A test program drove the shipped LogByteSizeMergePolicy, LogDocMergePolicy and TieredMergePolicy classes over every index of 2 to 4 segments from a small grid of sizes and delete fractions, with every non-empty proper subset of the segments marked as already merging, and checked rules such as "after forceMergeDeletes every segment that is not merging is free of deletes".
• Result. That rule failed 20,384 of 100,800 checks for each LogMergePolicy and 0 of 148,331 for TieredMergePolicy; a companion rule (no proposed merge may contain a segment that is already merging, which registerMerge would reject) failed 49,408 times per LogMergePolicy. With the one-line fix: 0 violations.
• Confirmation. A JUnit test on the stock jars (a gated scheduler holding the natural merge pending, and a latch holding it running in ConcurrentMergeScheduler), then independent verifiers (section "Prior reports, history and checks"), then this minimal test.