An IndexWriter opened from a leaf-sorted reader's commit applies deletes to the wrong segment

When an IndexWriter is opened from a reader's commit (IndexWriterConfig#setIndexCommit(reader.getIndexCommit())), it pre-enrolls the reader's segment readers and pairs them with its own segment list by position. A leaf sorter reorders the reader's leaves but not the segment list, so each segment gets another segment's reader. Deletes, updates and merges then act on the wrong documents and the next commit saves that, with CheckIndex clean. In the repro, deleteDocuments(id:a) deletes b and keeps a. Every release from 8.9.0, the first with a leaf sorter, through 10.6.0 and main is affected; pairing by segment name fixes it.

1. IndexWriter.java:982-989: the writer takes config.getIndexCommit(). A commit from DirectoryReader#getIndexCommit is a ReaderCommit (StandardDirectoryReader.java:571-576) whose package-private getReader() returns the reader itself (:644-646). A commit from listCommits is a ReaderCommit built with a null reader (DirectoryReader.java:456, :481), so the same getReader() returns null; that path never pre-enrolls anything and is not affected.
2. IndexWriter.java:1021-1050: in the reader branch ("Init from an existing already opened NRT or non-NRT reader", :1030), the writer clones reader.segmentInfos, which is in segment order [_0, _1] (:1050), and hands the reader to new ReaderPool(...) (:1149).
3. BaseCompositeReader.java:75-76: Arrays.sort(subReaders, subReadersSorter). The same reader's leaves are in sorter order [_1, _0].
4. ReaderPool.java:91-115 pre-enrolls the reader's segment readers "so any in-memory NRT live docs are correctly carried over, and so NRT readers pulled from this IW share the same segment reader". It pairs them by position. The only check that the two lists match is a size assert; the per-pair checks are in step 5:

List<LeafReaderContext> leaves = reader.leaves();          // [_1, _0]
assert segmentInfos.size() == leaves.size();               // ReaderPool.java:95
for (int i = 0; i < leaves.size(); i++) {                  // :96
  SegmentReader segReader = (SegmentReader) leaves.get(i).reader();
  SegmentReader newReader = new SegmentReader(segmentInfos.info(i), segReader, ...);  // :99-106
  readerMap.put(newReader.getOriginalSegmentInfo(), new ReadersAndUpdates(...));

So _0's pool entry gets _1's reader (document b) and _1's entry gets _0's reader (document a).

1. SegmentReader.java:128-152: the copy constructor takes its identity from si and its data from sr (this.core = sr.core, :151). It rejects only numDocs > maxDoc (:136-138) and a live-docs length mismatch (:139-142). An equal-size swap passes silently; an unequal one throws. Within a cycle of the permutation, unequal sizes always put a bigger reader on a smaller segment somewhere, so a swap passes these two guards exactly when the moved segments have equal maxDoc (a Z3 model of the two guards for 2 to 8 segments agrees). Equal sizes are needed for the silent case but not always enough: with a committed deletion in a swapped segment, the next commit throws (below).
2. deleteDocuments(id:a) is resolved against the pooled readers. Under _0 it finds nothing (that reader holds b); under _1 it finds doc 0 (that reader holds a) and marks doc 0 of _1 deleted. That delete is in memory, but it makes _1, which holds b on disk, fully deleted, so the commit drops _1 (IndexWriter.java:6750-6756): the new commit lists only _0, with a, no live-docs file is written, and CheckIndex is clean.

• updateDocument(id:a) deletes b and leaves two copies of a; updateNumericDocValue(id:a) sets b's value; softUpdateDocument(id:a) soft-deletes b.
• forceMerge(2) of three one-document segments a, b, c commits [a, a, b] (LogDocMergePolicy, merge factor 2, minMergeDocs 1): one document lost, one duplicated, CheckIndex clean. With the default minMergeDocs the merge takes all three segments and nothing is lost.
• From a sorted near-real-time reader that carries one buffered delete, a bare commit() revives the deleted document and deletes a live one, while the new writer's own NRT reader still shows the right documents.
• With equal sizes and a committed deletion in one of the swapped segments, the next commit throws NullPointerException (-da) or AssertionError (-ea) in PendingDeletes.writeLiveDocs. (Separately, PendingDeletes.java:41-44 counts the other segment's deletions as pending deletes of this one.)
• An NRT reader pulled from the new writer shows the right documents, but each leaf carries the other segment's name (_0 holds b, _1 holds a). Through that reader, tryDeleteDocument on a's doc id deletes b, and tryUpdateDocValue on it sets b's value: both resolve the segment from the leaf's own segment info (tryModifyDocument, IndexWriter.java:1791-1812), which here is the wrong name. Through the original sorted reader they return -1 and change nothing.
• Not affected: forceMerge(1) and a commit from listCommits.

• Who is hit. Applications that use a leaf sorter, through DirectoryReader.open(Directory, Comparator), the commit overload (DirectoryReader.open(IndexCommit, Comparator) in 8.10.x-8.11.x, open(IndexCommit, int, Comparator) from 9.0.0) or IndexWriterConfig#setLeafSorter for near-real-time readers (the javadoc suggests it for time-based indices: "usually a descending sort on timestamp"), and that open a writer with setIndexCommit(reader.getIndexCommit()), for example to roll back to a reader's point in time or to hand a near-real-time reader over to a new writer. The documented NRT hand-off, combined with the documented setLeafSorter option on both writers, fails the same way: id:a=1 id:b=0 instead of id:a=0 id:b=1. How often applications open a writer from the commit of the same leaf-sorted reader is unknown.
• Silent and durable. No exception; the commit persists the wrong deletion; CheckIndex reports the index clean, because it is self-consistent, just wrong.
• No scale needed. Two one-document segments. The test configures no codec, directory or merge policy (the test framework randomizes codec and directory; it failed on every seed tried), and a main() probe with the default codec gives the same result; assertions on and off.
• Loud otherwise. When the moved segments differ in size, the writer constructor throws IllegalArgumentException: numDocs=N but maxDoc=M (or maxDoc=M but liveDocs.size()=N), so the documented path cannot open that index at all.
• Upstream tests did not see it. TestIndexWriterFromReader never uses a leaf sorter, and TestIndexWriterReader.testIndexReaderWriterWithLeafSorter never opens a writer from a reader's commit. With a newest-first sorter injected into every config and reader open, TestIndexWriterFromReader.testRandom fails on 3 of 3 seeds (numDocs=N but maxDoc=1) and passes with the fix.
• How likely. Rare: both options are opt-in, the silent case needs equal sizes, the Elasticsearch, OpenSearch and Solr call sites of setIndexCommit read for two related bugs pass commits that carry no reader (code reading, not run), and no report was found in the five years since 8.9.0.

Copy the file into lucene/core/src/test/org/apache/lucene/index/ and run the command. The test sits in org.apache.lucene.index like TestIndexWriterFromReader but uses nothing package-private. The reversed segment-name comparator is the smallest deterministic leaf sorter; on these two segments it gives the same order as a newest-first timestamp sorter (an earlier version of the test used one, with a ts doc value, and fails the same way). newDirectory() may wrap the directory in a MockDirectoryWrapper, whose close runs CheckIndex; it passes.

This is the no-noise gate's minimal repro with one line wrapped by google-java-format, so that :lucene:core:check passes with it; nothing else differs. Pass -Ptests.asserts explicitly, because by default the build picks it from the seed. The test is also the suggested regression test as it stands.

./gradlew :lucene:core:test --tests "org.apache.lucene.index.TestIndexWriterFromLeafSortedReader" -Ptests.seed=DEADBEEF -Ptests.asserts=true

# gradle, main 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (the same with false)
TestIndexWriterFromLeafSortedReader > testDeleteThroughWriterOpenedFromLeafSortedReaderCommit FAILED
    java.lang.AssertionError: expected:<0> but was:<1>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:275702B30527020B]:0)
        ... (JUnit Assert frames)
        at org.apache.lucene.index.TestIndexWriterFromLeafSortedReader.testDeleteThroughWriterOpenedFromLeafSortedReaderCommit(TestIndexWriterFromLeafSortedReader.java:52)
:lucene:core:test (FAILURE): 1 test, 1 failure

# with the recommended fix, -Ptests.asserts=true:
:lucene:core:test (SUCCESS): 1 test
BUILD SUCCESSFUL

# with the fix, the test plus TestIndexWriterFromReader and TestReaderPool, -Ptests.asserts=true and false:
:lucene:core:test (SUCCESS): 19 tests
BUILD SUCCESSFUL

• Contract. IndexWriter.java:1856: "Deletes the document(s) containing any of the terms."
• The entry point names this flow. IndexWriterConfig.java:235-237: "Expert: allows to open a certain commit point. The default is null which opens the latest commit point. This can also be used to open IndexWriter from a near-real-time reader, if you pass the reader's DirectoryReader#getIndexCommit." The LUCENE-6524 CHANGES entry: "IndexWriter can now be initialized from an already open near-real-time or non-NRT reader." The leaf sorter is documented only as a search-order option (DirectoryReader.java:76-90; IndexWriterConfig.java:518-520: "A DirectoryReader opened from a IndexWriter with this configuration will have its leaf readers sorted with the provided leaf sorter."). No javadoc, CHANGES or MIGRATE entry, code comment or test restricts the combination.
• History. The positional loop came with LUCENE-6524 (2d0ac9bb7f2, 2015, #7582), when a reader's leaves always followed segment order; the size assert shows the assumption. LUCENE-8260 (2018) moved it into ReaderPool unchanged. LUCENE-9507 (48715fe898b, 2021, #10546 / #32) made leaves sortable and changed neither ReaderPool.java nor IndexWriter.java.
• Sibling code pairs by identity. Reader reopen maps old readers by segment name (StandardDirectoryReader.java:249-260), and tryDeleteDocument / tryUpdateDocValue resolve the segment from the leaf reader's own segment info (tryModifyDocument, IndexWriter.java:1791-1812).
• Why tests never saw it. In the first review of LUCENE-9507 (apache/lucene-solr#2256, 2021-02-09), jimczi suggested a random leaf sorter in LuceneTestCase#newIndexWriterConfig; the author replied that a different order "will break some of our tests that rely on specific docIds", so it was not done.
• Precedent. LUCENE-8466 (2018), "IndexWriter.deleteDocs(Query... query) incorrectly applies deletes on flush if the index is sorted", treated deletes landing on the wrong documents as a bug.

Every API involved is marked advanced: setIndexCommit is "Expert", DirectoryReader#getIndexCommit is "Expert" and @lucene.experimental, IndexCommit is @lucene.experimental, and on #214 jpountz called leafSorter one of the "super expert parameters". So a maintainer could call the combination unsupported.

Why it fails: no such precondition is documented; setIndexCommit names exactly reader.getIndexCommit(); setLeafSorter is an IndexWriterConfig option, so the documented NRT hand-off combined with that documented option yields a sorted reader without any misuse, and it fails the same way; "experimental" lets the API change, it does not let deleteDocuments delete a document that lacks the term and commit that silently. The executing reviews from another model family and the independent reproducer rated the intended-behaviour case weak; the devil's advocate found that none of its points survives; the adjudication agreed, and the audit found that deleting a document without the term "cannot be intended".

One fix was built and validated; the other options come from the reviews and the brief and were not run.

In the ReaderPool constructor, map the reader's leaves by SegmentReader.getSegmentName() and look up each SegmentCommitInfo by info.info.name, instead of taking leaves.get(i). +10/-3 lines in ReaderPool.java; Map and HashMap are already imported.

// ReaderPool constructor, reader != null: pair by segment name, not by position
Map<String, SegmentReader> readersByName = new HashMap<>();
for (LeafReaderContext leaf : leaves) {
  SegmentReader segReader = (SegmentReader) leaf.reader();
  readersByName.put(segReader.getSegmentName(), segReader);
}
for (SegmentCommitInfo info : segmentInfos) {
  SegmentReader segReader = readersByName.get(info.info.name);
  assert segReader != null : "no reader for segment " + info.info.name;
  SegmentReader newReader =
      new SegmentReader(info, segReader, segReader.getLiveDocs(),
          segReader.getHardLiveDocs(), segReader.numDocs(), true);
  // readerMap.put(...) unchanged
}

Trade-offs

• One HashMap per writer opened from a reader: linear, once, in the constructor.
• Unchanged for unsorted readers, where position and name agree. It is the rule reader reopen already uses (StandardDirectoryReader.java:249-260).
• The pool's map is keyed by SegmentCommitInfo, so the insertion order does not matter.

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF: stock, the repro fails with assertions on and off while TestIndexWriterFromReader (12) and TestReaderPool (6) pass; with the fix, all 19 tests pass with assertions on and off.
• With the fix, assertions on: TestIndexWriterReader (24, one assumption skip), TestDirectoryReader (34) and TestDirectoryReaderReopen (30) pass; :lucene:core:check -x test (format, ecj lint, forbidden APIs) passes with the fix and the test.
• Earlier, in the repro harness: the independent reproducer's and both reviewers' tests pass; the 5,220-run bounded-exhaustive differential test matches the no-sorter control in every run; with a newest-first sorter injected, TestIndexWriterFromReader passes 12 of 12 on 3 seeds; TestSoftDeletesDirectoryReaderWrapper (5) and TestSoftDeletesRetentionMergePolicy (14) pass.
• Not run: the full Lucene test suite. The focused suites are enough for an issue and a candidate patch; run the full suite before calling the patch ready to merge.

Replace the assert segReader != null with an IllegalStateException, as one of the reviews suggested.

Trade-offs

A missing name cannot happen when the segment list and the leaves come from the same reader (the NRT path drops fully deleted segments from both), so this only changes how an internal inconsistency would surface. Not run.

Throw IllegalArgumentException in the writer's reader branch when the reader's leaves are not in segment order.

Trade-offs

Small and safe, but it rejects a documented composition of options (rollback or NRT hand-off with a leaf sorter) that today either corrupts silently or fails loudly, instead of making it work, and the javadoc would have to say so. Not run.

Have StandardDirectoryReader or its ReaderCommit keep the segment readers in segment order next to the sorted view, and pre-enroll from that.

Trade-offs

Equivalent to pairing by name, but it changes reader internals for a constructor-local problem. Not run.

Setting a random setLeafSorter in LuceneTestCase#newIndexWriterConfig, as jimczi proposed on apache/lucene-solr#2256, would have exposed this through TestIndexWriterFromReader.testRandom.

Trade-offs

Coverage, not a fix; many tests depend on doc ids, which is why it was not adopted in 2021. The regression test for this bug is the one repro test.

• Lead (non-formal). A planning pass of the hunt's round 18 proposed it and witnessed it with public-API main() probes on main: one with a directory reader opened with a newest-first sorter, one with setLeafSorter on both writers and a near-real-time reader. A skeptic pass re-ran it, widened it (forceMerge(2) with minMergeDocs 1 commits [a, a, b]) and found it on 8.9.0, 8.10.0 and 9.0.0. In the same round, injection replays (a newest-first sorter switched on in copied upstream test classes) hit the same root through TestIndexWriterFromReader.testRandom.
• Hunt. The unit reduced it to one public-API JUnit test. Then two formal legs: a Z3 model of the writer constructor's two guards (2 to 8 segments) separates the leaf orders that pass silently (equal maxDoc) from those that fail loudly; a bounded-exhaustive differential test on the real jar (5,220 runs: 2 or 3 segments of 1 or 2 documents, with and without a deletion, every leaf order, directory and near-real-time readers, no-op, delete, update, doc-values update, forceMerge(1) and forceMerge(2); oracle: the same steps from the same commit without a sorter) matched the model's prediction in every run, and is 5,220 of 5,220 equal to the oracle with the fix.
• Because the formal legs came after the lead, the hunt counts the bug as found by non-formal means.