BpVectorReorderer's in-place command-line reorder commits an empty or partial index over the original when reordering fails

BpVectorReorderer (lucene/misc, added for 11.0 by #14097) has a command line, main(indexPath, field), that renumbers the documents of an existing index in place so that documents with similar vectors get nearby doc ids. It calls the package-private reorderIndexDirectory, which opens a reader on the index, then an IndexWriter in OpenMode.CREATE on the same directory inside try-with-resources, and adds each segment back reordered with one addIndexes call per segment, with no catch and no rollback. If computing a doc map or addIndexes throws, for example the documented NotEnoughRAMException when a segment's doc-map metadata (8 bytes per vector) reaches the RAM budget (by default a tenth of the JVM's current heap), try-with-resources calls close(), which commits by default. The new commit holds only the segments added before the failure, and the default deletion policy deletes the original commit. In the minimal test 1 document becomes 0; through main() with default options 601,000 documents became 1,000 in a small-heap JVM, and indexes written by Lucene 9.12.3 or 10.5.2, or with soft deletes or a parent field, became empty. Unreleased: the tool is on main and on the 11.0 release branch branch_11x, and in no release, so it can be fixed on both before 11.0.0.

1. main (BpVectorReorderer.java:653-682) takes the index path, the field and three tuning options (--max-iters, --min-partition-size, --thread-count); it has no option for the RAM budget or the writer configuration. It calls reorderIndexDirectory(dir, executor) on FSDirectory.open(...) (:679-681).
2. The RAM budget is set in the constructor to a tenth of Runtime.getRuntime().totalMemory() (:97), the JVM's current heap, which early in main() is about the initial heap. computeValueMap throws NotEnoughRAMException when a segment's vectors, at 8 bytes each (:642-646), need at least that budget (>=, :596-605); computeDocMap (:578-593) runs once per segment.
3. reorderIndexDirectory (:684-697) opens a reader on the directory, then try (IndexWriter writer = new IndexWriter(directory, iwc)) with OpenMode.CREATE and an otherwise default IndexWriterConfig, and calls writer.addIndexes(SortingCodecReader.wrap(leaf, computeDocMap(leaf, null, executor), null)) for each segment. Each addIndexes call is transactional on its own (IndexWriter.java:3084-3086), not the loop.
4. When computeDocMap throws (NotEnoughRAMException; an AssertionError under -ea) or addIndexes throws (an IllegalArgumentException for a 9.x/10.x-created, soft-deletes or parent-field index; an IOException), try-with-resources calls close(). With commitOnClose (IndexWriterConfig.java:102) close() writes, waits for merges and commits (IndexWriter.java:1363-1369).
5. The new commit, segments_2, lists only the segments added before the failure; the default KeepOnlyLastCommitDeletionPolicy deletes the original commit and its files. After the 9.12.3, 10.5.2, soft-update and parent-field runs the directory holds only segments_2 and write.lock.
6. A failure that also breaks the commit (a disk that stays full) keeps the old index, because the commit itself fails. So the loss needs a failure before the commit that does not recur in it, which is what NotEnoughRAMException and the IllegalArgumentExceptions are.

The 9.x/10.x and soft-deletes failures are the IllegalArgumentException of the sibling bug LFV-1023 (the tool's default IndexWriterConfig refuses those segments); refusing a parent-field index is arguably right, since reordering would break the blocks, but deleting it is not. An index that only configured a soft-deletes field, with no soft delete recorded, reorders fine (11 -> 11).

• Who is exposed. Users of unreleased main or the 11.0 release branch branch_11x (11.0.0-SNAPSHOT) who run BpVectorReorderer's command line on an index, or call reorderIndexDirectory from the same package. Applications that use BPReorderingMergePolicy or computeDocMap with their own IndexWriter are not affected.
• Default vs opt-in. The tool is opt-in, an expert tool run by hand. Once it runs, everything is default, and it has no option that avoids the loss.
• How often a run fails. With default options the tool's writer refuses every segment of a non-empty index created by Lucene 9.x or 10.x and every segment that carries a soft-deletes or parent field (the sibling bug LFV-1023 and the parent-field check); in every such run it was the first segment, and the index was wiped to 0 documents (executed on small non-empty indexes; the general form is read from addIndexes' per-segment checks). Independently, the budget runs out on a segment whose vectors need at least the budget at 8 bytes each. HotSpot's default initial heap is 1/64 of physical memory (InitialRAMPercentage 1.5625, printed on the test host), which gives about 3.4 million vectors on a 16 GB machine (computed, not run); other JVMs, container limits or -Xms change it. The tool reorders each segment as a whole, so the limit is hit by large segments, such as those of a force-merged index (reasoning; upstream's dense-index test force-merges to one segment before calling main()).
• Effect. Durable: the original commit is gone, and only the exception tells the user. When a later segment fails, the partial index opens without error (12 of 52 documents). Recovery needs a backup or a re-index.
• Mitigating. Unreleased, offline, and loud (an exception). A user who copies the index first loses nothing.

One test class, one method, single-threaded (executor null), formatted by the module's tidy task. It is the hunt's minimal repro with its comment reworded to fit one line. The seed randomizes the test environment (codec and per-field formats, similarity, locale, time zone, two size settings); the failure reproduced with every seed tried: gradle (seed 5EED1022) and the standalone runner (seeds DEADBEEF, FC1022A1, FC1022A2), assertions on and off, and the finder's 10-document version on six seeds.

expectThrows passes, so the reorder did throw NotEnoughRAMException; the failing assertion is the document count. The public main() path (601,000 -> 1,000; 9.x/10.x, soft-deletes and parent-field indexes -> 0) was executed separately and is in the explanation above.

cp $REPRO/TestBpVectorReorderFailureKeepsIndex.java $LUCENE/lucene/misc/src/test/org/apache/lucene/misc/index/
cd $LUCENE && ./gradlew :lucene:misc:test --tests TestBpVectorReorderFailureKeepsIndex -Ptests.seed=5EED1022
# the same failure with -Ptests.asserts=false

TestBpVectorReorderFailureKeepsIndex > testFailedReorderKeepsIndex FAILED
    java.lang.AssertionError: expected:<1> but was:<0>
        at __randomizedtesting.SeedInfo.seed([5EED1022:6D0CCC9A5B975BAC]:0)
        ...
        at org.apache.lucene.misc.index.TestBpVectorReorderFailureKeepsIndex.testFailedReorderKeepsIndex(TestBpVectorReorderFailureKeepsIndex.java:44)
1 test completed, 1 failed

# the same message with -Ptests.asserts=false, and with the standalone runner (assertions on and off); passes with the fix

• Contract. No javadoc promises failure atomicity in so many words. main: "a path containing an index to reorder" (BpVectorReorderer.java:648-651), a permutation of its documents; the upstream test calls it "the index reordering tool" (TestBpVectorReorderer.java:353, :412). setRAMBudgetMB: "If not enough RAM is provided, a {@link NotEnoughRAMException} will be thrown" (AbstractBPReorderer.java:63-67).
• The sibling handles the same error without damage. BPReorderingMergePolicy: "When reordering doesn't have enough RAM, it simply skips reordering in order not to fail the merge" (BPReorderingMergePolicy.java:34-39); testReorderDoesntHaveEnoughRAM uses the same Double.MIN_VALUE budget and "makes sure that reordering the index on merge does not corrupt its content" (TestBPReorderingMergePolicy.java:248-251).
• IndexWriter semantics. A CREATE writer replaces the old index only when it commits: old readers "won't see the newly created index until they re-open" (IndexWriter.java:103-107); addIndexes "does not commit a new segments_N file until all indexes are added" (:3084-3086). The half-added commit is made by the tool's own try-with-resources.
• Lucene's other tools. CheckIndex -exorcise: "LOSES DATA" and "Always make a backup" (CheckIndex.java:4645, :4652, :4856); IndexSplitter: "be careful!" (IndexSplitter.java:44). The in-place IndexUpgrader also closes its writer in try-with-resources, but on the existing index (default CREATE_OR_APPEND), so a commit after a failure still holds every document (read, not executed). BpVectorReorderer carries no warning, no @lucene.experimental and no backup advice.
• Tests and history. Upstream tests call reorderIndexDirectory (TestBpVectorReorderer.java:175) and main (:354, :413) on successful runs only. The comments and reviews of #13565, #13683, #14097 and #14117 never discuss a failing run.

Strongest case (the refute-lens cross-model review rated it moderate; the devil's-advocate check built it in full and rated it weak): it is a research tool (in #13565 its author wrote "it would be easier to start with a tool that rewrites the index" to get an idea of the impact; CHANGES never mentions the command line, and even a successful run drops commit user data and any index sort, executed as a side observation and not claimed); OpenMode.CREATE is documented as "Creates a new index or overwrites an existing one" (IndexWriterConfig.java:58); close() committing is documented and was restored as the default on purpose (LUCENE-5871); and the minimal test needs the package-private method and a budget a command-line user cannot set.

Why it fails: nothing says research-only or "make a backup", main() is public in lucene/misc (not the sandbox), and Lucene's data-losing tools warn loudly. Granted, CREATE is documented to overwrite and an IndexWriter caller normally owns rollback; but here the tool picks CREATE and owns the writer's whole lifecycle, so the tool is the caller that should roll back. What CREATE is meant to put in place is the reordered index; the empty or partial commit is made by the tool's own try-with-resources after it has thrown. commitOnClose leaves rollback to the caller, and the caller here is Lucene's own tool, which gives its user no hook. Public main() with default options reaches it on ordinary indexes. Verdict: a defect. Residual risk: a maintainer may prefer to remove main() or document "back up first".

Option A was built and validated with gradle in a scratch worktree at 645d4fe14a20; options B, C and D were not built.

One method, reorderIndexDirectory in BpVectorReorderer.java (+12/-1): open the writer without try-with-resources; if adding any segment throws, call writer.rollback(), which discards the added segments and keeps the original commit, add a rollback failure as suppressed and rethrow; call writer.close() only after the loop. The logic is the finder's fix sketch with the comment reworded.

IndexWriter writer = new IndexWriter(directory, iwc);
try {
  for (LeafReaderContext ctx : reader.leaves()) {
    CodecReader codecReader = (CodecReader) ctx.reader();
    writer.addIndexes(
        SortingCodecReader.wrap(codecReader, computeDocMap(codecReader, null, executor), null));
  }
} catch (Throwable t) {
  try { writer.rollback(); } catch (Throwable t2) { t.addSuppressed(t2); }
  throw t;
}
writer.close();

Trade-offs

• The success path is unchanged: close() still flushes, waits for merges and commits.
• On failure the writer is rolled back and closed and the original exception is rethrown. Throwable is caught so an AssertionError (under -ea) is covered too.
• It does not fix LFV-1023: the tool still refuses 9.x/10.x and soft-deletes indexes, now without deleting them. Indexes already damaged are not restored.
• Open PR #15430 moves the class to core with this method unchanged; if it merges first, the fix moves with it.
• Validated for failures inside the segment loop (computeDocMap, addIndexes), where every observed trigger fires. Not tested: the IndexWriter constructor failing, rollback() itself failing, or close() failing after every segment was added (its javadoc: on an exception "changes may have been lost").

What the checks validated

• Scratch worktree at 645d4fe14a20, gradle, JDK 25: the test fails on stock and passes with the fix, assertions on and off.
• ./gradlew :lucene:misc:check with the fix: formatting (checkGoogleJavaFormat), ecjLint, forbiddenApis and the whole module's tests (24 suites, 317 tests, 0 failures, 9 skipped), including TestBpVectorReorderer (8 of 8) and TestBPReorderingMergePolicy (its two @Nightly tests were skipped; they do not call the changed method). With assertions off the three reorder test classes pass too.
• Earlier, the same logic passed the finder's 10-document test, the upstream reorder tests (9 of 9) and the bounded-exhaustive harness with 0 violations of 483 checks (stock: 236).

Two lines: with commitOnClose off, close() means rollback() (IndexWriter.java:1363-1369), so any failure before the explicit commit keeps the old index.

Trade-offs

On success, merges still running after the explicit commit are aborted by the rollback in close() instead of being waited for and committed (addIndexes(CodecReader...) calls maybeMerge(), IndexWriter.java:3395), so the result may keep more segments than today (inferred, not run).

Leave the input alone, or swap the new index in only after a successful commit.

Trade-offs

The safest for the input, but it changes the command line's in-place contract and needs twice the disk space.

Document "back up the index first; a failed run can leave it empty or partial".

Trade-offs

No code risk, but the loss remains, and today's most common failures happen on ordinary indexes.

Method: code reading plus executed probes, then a bounded-exhaustive harness (non-formal tier: the first signal was code reading and executed probes, not a model).

• A failure-atomicity planning pass read reorderIndexDirectory (try-with-resources around an in-place CREATE writer, default commitOnClose) and ran probes on stock main: a single injected I/O fault at each of the 225 directory calls of an in-place reorder committed a partial or empty index at 207, and a stock main() run in a JVM with a small initial heap turned 601,000 documents into 1,000. A skeptic pass reduced it to a first-segment failure.
• The hunt then checked the real jars with a bounded-exhaustive harness against an executable failure-atomicity reference: all 30 small/over-budget layouts of 1 to 4 segments, every fault point one-shot and sticky, and three natural triggers (an index created by an earlier major, a soft-deletes field, a parent field). Stock: 236 violations of 483 checks; with the fix: 0.
• It wrote a 10-document JUnit, later reduced to the 1-document test above. The sibling refusal of 9.x/10.x and soft-deletes indexes (LFV-1023) was filed separately.