With assertions on, re-sorting an index with document blocks through SortingCodecReader fails its cache assertion for the parent field

SortingCodecReader's javadoc documents how to re-sort an existing index: wrap every segment with SortingCodecReader.wrap(reader, sort) and add the wrapped readers to a fresh IndexWriter with addIndexes(CodecReader...). For an index with document blocks and a parent field (IndexWriterConfig#setParentField), that re-sort throws AssertionError: only sort fields should be cached twice but [_parent] is not a sort field with assertions enabled (tested: a block of several documents, a wrap that reorders documents, and a parent field that is not in the sort). The wrapped reader caches only the last doc-values field it rebuilt, and its assertion allows only sort fields to be rebuilt twice; since #12829 (9.10.0) the merge reads the parent field twice, in MultiSorter.sort and in the doc-values merge, and the allow-list was never extended. With assertions off the re-sort completes and the result is correct. Every release from 9.10.0 to 10.6.0 and main is affected; the practical effect is that no passing test of the documented recipe on block indexes can run with assertions on.

1. The wrap computes the new order with Sorter.sort(LeafReader), which since #12829 reads the parent field of the unwrapped segment and sorts each document by its block's parent (Sorter.java:185-199, called from SortingCodecReader.java:446). In the example, c1, p1, x becomes x, c1, p1.
2. Every doc-values read through the wrapped reader rebuilds the field in the new order (getNumeric -> getOrCreateDV, SortingCodecReader.java:713-716), in the reader's own getNumericDocValues (:793-804): always a long[maxDoc] and a FixedBitSet(maxDoc).
3. A single-entry cache keeps only the last rebuilt field (SortingCodecReader.java:864-885). Its comment accepts rebuilding sort fields: "we do the work twice for these fields" (:866).
4. The assertion counts rebuilds and allows a second one only for a field of the reader's sort (:890-918); otherwise "only sort fields should be cached twice but [_parent] is not a sort field" (:914-915). Its comment: "this is mainly there to make sure we change anything in the way we merge we realize it early" (:892).
5. Rebuild 1 of _parent: addIndexes(CodecReader...) merges the wrapped reader. The merge state calls MultiSorter.sort (MergeState.java:223), which reads the parent field of every reader with blocks (MultiSorter.java:57-63), then the sort field (:84), so the cache now holds sort. (This is the order since 10.5.0; older releases read the sort field first.)
6. Rebuild 2 of _parent: the doc-values merge asks for every numeric field again (DocValuesConsumer.java:204-216, getNumeric at :215). _parent is not cached and is not a sort field, so the assertion fails. AssertionError is an Error, so addIndexes treats it as a tragic event and the destination writer closes (IndexWriter.java:3390-3392).
7. With assertions off step 4 does not run: _parent is simply rebuilt twice and the result is correct.

The cache and the sort-only allow-list come from LUCENE-9539 (2020); the parent-field read in MultiSorter from #12829 (2024), which did not touch SortingCodecReader. A pull-counting wrapper below the reader confirmed, after the wrap, the two reads of _parent (MultiSorter.sort:58, DocValuesConsumer.getMergedNumericDocValues:215) and a single read of the sort field (by MultiSorter); during the wrap, Sorter.sort had read each field once (step 1).

• What they lose: the re-sort job (the destination writer closes, its last commit intact; the source is only read). No passing regression test of the recipe on block indexes can run on stock code with assertions on: the hunt's own differential test had to skip its SortingCodecReader build for block indexes under -ea because of this bug.
• Opt-in throughout: index sorting, setParentField, document blocks, the @lucene.experimental SortingCodecReader and assertions on. Assertions are off by default in the JVM.
• Not affected: IndexWriter's own index-sorted merges of block segments, the recipe without blocks, and readers already in the target order (the wrap then returns a plain FilterCodecReader, SortingCodecReader.java:458-481). BPIndexReorderer also wraps with SortingCodecReader.wrap(reader, docMap, null) but is not affected as Lucene uses it: merge-time reordering is skipped under an index sort (IndexWriter.java:3560, :5555), and without an index sort MultiSorter never runs (MergeState.java:216-223; read, not run).
• Not claimed: wrong results, corruption, a hang, or a failure in a default production JVM (assertions off); a JVM started with -ea would lose the destination writer. With assertions off the parent field is rebuilt one extra time per wrapped reader; that cost was not measured.

The test sits in org.apache.lucene.index next to TestSortingCodecReader but uses no package-private API. The source index has the parent field _parent and no index sort: one block [c1, p1] with sort=7, then a single document x with sort=3. The wrap must reorder (x, c1, p1); an already-sorted reader does not trigger the bug. SerialMergeScheduler keeps the failure on the test thread; with the default ConcurrentMergeScheduler the assertion fires in a merge thread, and in the adjudication's run the test framework reported both the AssertionError and an uncaught MergePolicy$MergeException wrapping it. The test only checks that the re-sort does not throw; the block-order checks with assertions off are in the hunt's and verifiers' tests. On main and 10.4.0 and later it fails in every run; on older releases add -Dtests.codec=<the release's default codec> (e.g. Lucene99 on 9.10.0) to make it fail every time. It passes the core lint as is. The regression test for a fix is this same test.

# gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF, -Ptests.asserts=true (environment lines cut)
TestSortingCodecReaderParentField > testResortBlocksWithAddIndexes FAILED
    java.lang.AssertionError: only sort fields should be cached twice but [_parent] is not a sort field
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:785B981BAB2A8772]:0)
        at org.apache.lucene.index.SortingCodecReader.assertCreatedOnlyOnce(SortingCodecReader.java:914)
        at org.apache.lucene.index.SortingCodecReader.getOrCreate(SortingCodecReader.java:879)
        at org.apache.lucene.index.SortingCodecReader.getOrCreateDV(SortingCodecReader.java:921)
        at org.apache.lucene.index.SortingCodecReader$7.getNumeric(SortingCodecReader.java:715)
        at org.apache.lucene.codecs.DocValuesConsumer.getMergedNumericDocValues(DocValuesConsumer.java:215)
        at org.apache.lucene.codecs.DocValuesConsumer$1.getNumeric(DocValuesConsumer.java:194)
        [...]
        at org.apache.lucene.index.SegmentMerger.mergeDocValues(SegmentMerger.java:192)
        at org.apache.lucene.index.SegmentMerger.mergeWithLogging(SegmentMerger.java:318)
        at org.apache.lucene.index.SegmentMerger.merge(SegmentMerger.java:153)
        at org.apache.lucene.index.IndexWriter.addIndexesReaderMerge(IndexWriter.java:3589)
        at org.apache.lucene.index.IndexWriter$AddIndexesMergeSource.merge(IndexWriter.java:3463)
        at org.apache.lucene.index.SerialMergeScheduler.merge(SerialMergeScheduler.java:38)
        at org.apache.lucene.index.IndexWriter.addIndexes(IndexWriter.java:3312)
        at org.apache.lucene.index.TestSortingCodecReaderParentField.testResortBlocksWithAddIndexes(TestSortingCodecReaderParentField.java:53)

# -Ptests.asserts=false: 1 test, 0 failures
# with fix A or fix B: 1 test, 0 failures, assertions on and off
# standalone runs (-ea, seed DEADBEEF): fails in every run on main, 10.6.0, 10.5.2 and 10.4.0; 10.6.0 passes with -da;
# on 9.10.0, 9.12.3 and 10.0.0 only in some runs under the random test codec, and in every run with
# -Dtests.codec=<the release's default codec>;
# 9.9.2: does not compile (no IndexWriterConfig#setParentField)

• The recipe is documented: "This can be used to re-sort and index after it's been created by wrapping all readers of the index with this reader and adding it to a fresh IndexWriter via IndexWriter#addIndexes(CodecReader...)" (SortingCodecReader.java:48-51). addIndexes throws "cannot change index sort" for segments without the target sort (IndexWriter.java:3158, :3245-3251).
• Blocks are meant to survive it: #12829 made Sorter.sort(LeafReader) parent-aware (Sorter.java:191-199); its only production caller is SortingCodecReader.wrap (:446). CHANGES for #12829: "Document blocks are maintained alongside their parent documents during sort and merge" (lucene/CHANGES.txt:2978-2982, the 9.10.0 entry; the 10.0.0 entry at :2311-2314 has the same sentence). setParentField says the parent marker "is required" under index sorting (IndexWriterConfig.java:580-589).
• The code contradicts itself: the cache comment accepts doing "the work twice" for fields the sort needs (SortingCodecReader.java:864-866), and MultiSorter reads the parent field for exactly that purpose. The allow-list (LUCENE-9539, 2020) predates the parent field; #12829's review never mentions SortingCodecReader.
• No test covers this trigger: no upstream test re-sorts a block index through SortingCodecReader.wrap(reader, sort) and addIndexes into an index-sorted writer. TestBPIndexReorderer sets a parent field (:162) and reaches SortingCodecReader.wrap(reader, docMap, null) through BPIndexReorderer, but with no index sort, so MultiSorter never runs.
• Precedent: #13479 fixed an NPE inside this assertion when it misfired on legal use; #13478 (open) treats the assertion firing under intra-merge threads as a real problem.

The strongest case for "intended" (the cross-model reviews, the devil's-advocate verifier and the adjudication): the assertion is a deliberate tripwire for changes in how merges read fields (SortingCodecReader.java:892), the reader is @lucene.experimental and "might be very costly and memory intensive" (:52), and with assertions off the output is correct.

Why it fails: the tripwire fired because #12829 changed the merge's access pattern, and the follow-up (allow the parent field, or stop rebuilding it) was never done. Until one of them changes, no assertion-enabled test can run the documented recipe on a block index. That argues for low severity, not for intended behaviour.

Both options were run through gradle in a scratch worktree of 645d4fe14a20. A is the smallest change and keeps today's design; B also removes the extra rebuild at the price of holding the parent field's values longer. Deleting the assertion is not a fix: it drops the tripwire LUCENE-9539 added on purpose.

In assertCreatedOnlyOnce, treat the parent field like a sort field: MultiSorter reads it to keep blocks together, and the doc-values merge reads it again.

// MultiSorter also reads the parent field, to keep document blocks together, before the doc
// values merge reads it again
boolean isParentField = field.equals(getFieldInfos().getParentField());
...
assert isSortField || isParentField
    : "only sort fields and the parent field should be cached twice but ["
        + field
        + "] is neither";

Trade-offs

• Smallest change, same design: the code already accepts rebuilding fields the sort needs (SortingCodecReader.java:864-866). The "at most twice" check stays, so a third rebuild is still caught.
• getParentField() may be null; field.equals(null) is false, so writers without a parent field behave as today.
• With assertions off nothing changes: the parent field is still rebuilt once more per wrapped reader (not measured).
• Backports: fine from 10.5.0 (one parent read per reader in MultiSorter, #16202). In 9.10.0-10.4.0, with two or more sort fields, MultiSorter reads the parent once per sort field, so the "at most twice" check would still fire there; such a backport needs more (not run).
• Scope: A fixes this single-threaded trigger only, not #13478, where intra-merge threads evict each other from the single-entry cache.

What the checks validated

• Gradle in a scratch worktree of 645d4fe14a20 (--offline, seed DEADBEEF). Stock: the repro fails with assertions on and passes with them off.
• With A: the repro plus TestSortingCodecReader, TestIndexSorting, TestAddIndexes and TestFilterReadersForwardVectorPrefetch pass with assertions on (5 suites, 109 tests, 0 failures); the repro passes with assertions off; the whole org.apache.lucene.index test package passes with assertions on (1396 tests, 0 failures, 56 skipped); TestBPIndexReorderer (8 tests) and TestIndexSortBackwardsCompatibility (68 tests) pass (assertions at gradle's seed default, on for seed DEADBEEF by calculation, not recorded in the logs; a third parent-field suite outside core, TestMockRandomMergePolicy, was not run); checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest pass for core with the repro in place.
• Earlier, the hunt's equivalent sketch passed the finder's, the independent reproducer's and the cross-model reviewers' tests and TestSortingCodecReader (3/3) on main ea57360912ed.

getOrCreateDV keeps the parent field's rebuilt values in a second slot next to the single-entry cache (+14 lines in SortingCodecReader.java); the assertion still counts that single rebuild.

private Object cachedParentObject;

private <T> T getOrCreateDV(String field, IOSupplier<T> supplier) throws IOException {
  if (field.equals(getFieldInfos().getParentField())) {
    synchronized (this) {
      if (cachedParentObject == null) {
        assert assertCreatedOnlyOnce(field, false);
        cachedParentObject = supplier.get();
      }
      return (T) cachedParentObject;
    }
  }
  return getOrCreate(field, false, supplier);
}

Trade-offs

• Also removes the extra rebuild with assertions off, in the spirit of the TODO in the cache comment (SortingCodecReader.java:864-866).
• The parent field's long[maxDoc] and FixedBitSet(maxDoc) stay referenced by the wrapped reader from MultiSorter.sort until the reader is released after the merge, so the peak memory of a re-sort can grow by that much. Not measured.

What the checks validated

• Gradle in the same scratch worktree: the repro plus TestSortingCodecReader, TestIndexSorting, TestAddIndexes and TestFilterReadersForwardVectorPrefetch pass with assertions on (5 suites, 109 tests, 0 failures); the repro passes with assertions off; checkGoogleJavaFormat, ecjLintMain and forbiddenApisMain pass.
• Only the repro exercises the new branch; the larger runs of A were not repeated for B.

Method: a randomized differential test, then code reading and JUnit (the hunt's "non-formal" tier).

• A hunt unit (index sorting with document blocks) ran a differential baseline test: the same random operations built unsorted, with IndexWriterConfig.setIndexSort, through SortingCodecReader.wrap of every leaf plus addIndexes, and in pre-sorted order, compared per stored id.
• In the runs with assertions on, every iteration with document blocks failed the SortingCodecReader + addIndexes build with this AssertionError. With assertions off the four builds agreed.
• The finder traced the failing frames to the cache assertion and the parent-field read in MultiSorter, and minimised it to a JUnit test on the stock jar; the hunt later reduced that to the one-method test above.