ReindexingEnrichedDirectoryTaxonomyWriter.reindexWithNewOrdinalData leaves a half-rebuilt taxonomy when the appender throws, and close() commits it

ReindexingEnrichedDirectoryTaxonomyWriter (facet module, @lucene.experimental, since 9.10.0) stores extra fields in the categories' taxonomy documents; to change them, the class javadoc says it is required to call reindexWithNewOrdinalData(appender), which promises "The ordinals themselves will be preserved, so the caller does not need to update references to them in the main index". The method records every category of the last commit, calls deleteAll() (which drops every taxonomy document from the writer, pending until the next commit, and resets the ordinal counter to 0), re-adds the categories one by one through the new appender, and commits only at the end, with no exception handling. If the appender throws for one category, the writer is left open with only the categories before it, and close(), which commits, makes that permanent. In the repro the committed taxonomy goes from 5 categories to 3 and d/b and d/c resolve to -1, while main-index documents still store their ordinals 3 and 4: facet counting throws ArrayIndexOutOfBoundsException, and a writer kept in use hands 3 and 4 to the next new categories, so documents are silently counted under the wrong labels. rollback() right after the exception keeps the last commit. Every release from 9.10.0 to 10.6.0 and main is affected; committing first and rolling back when the rebuild fails fixes it.

• Taxonomy and ordinal. Taxonomy facets keep each label, such as d/b, once in a side index; its document number there is the label's ordinal. FacetsConfig.build asks the taxonomy writer for it (addCategory, FacetsConfig.java:383) and stores it in the main-index document as a doc-values field (:412). Ordinals start at 0 (the root) and grow by one per new label.
• Ordinal data and the appender. ReindexingEnrichedDirectoryTaxonomyWriter lets you add fields to each label's taxonomy document through an appender, a BiConsumer<FacetLabel, Document>. To change those fields you call reindexWithNewOrdinalData(appender).
• close() commits. DirectoryTaxonomyWriter.close() "commits whatever changes made to it"; rollback() discards them and closes the writer.

1. reindexWithNewOrdinalData (ReindexingEnrichedDirectoryTaxonomyWriter.java:87-104) installs the new appender (:90), records all labels in ordinal order from the last commit (:94), then calls deleteAll() (:97). deleteAll() (DirectoryTaxonomyWriter.java:902-919) drops every taxonomy document from the writer (:903; IndexWriter.deleteAll "will not be visible until a commit() has been called", IndexWriter.java:2731-2733), resets the ordinal counter to 0 (:907) and clears the cache (:912).
2. The loop (:100-102) re-adds each label with addCategory. For each one, addCategoryDocument calls the appender (enrichOrdinalDocument, DirectoryTaxonomyWriter.java:466; ReindexingEnrichedDirectoryTaxonomyWriter.java:50-54) before indexWriter.addDocument (:468) and the counter increment (:469).
3. commit() is the last statement (:103), and there is no try. When the appender throws on d/b, it has been called for root, d, d/a and d/b; the writer holds 3 labels, its getSize() is 3, and it is still open. The last commit on disk is still intact; the damage is pending in the writer.
4. close() calls commit() before closing (DirectoryTaxonomyWriter.java:288-294), so try-with-resources, a finally or IOUtils.close(...) persists the 3-label taxonomy. Main-index documents keep 3 and 4: lookups give -1 and counting throws. A writer kept open continues from counter 3, so the next new labels get 3 and 4 and those documents are counted under them.
5. The root trap. The base constructor adds the root (DirectoryTaxonomyWriter.java:181-185) inside super(d), before the subclass stores the appender (ReindexingEnrichedDirectoryTaxonomyWriter.java:44-45), so normal indexing never shows the root to the appender. A reindex re-adds it first, as a label of length 0. An appender that reads label.components[label.length - 1] then throws ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0, nothing is re-added, and close() commits size 0. Lucene's own test appender has this shape and guards it (TestOrdinalData.java:65-67).
6. Not I/O faults. In a bounded-exhaustive enumeration, each of the 269 directory calls of a successful reindex was failed in turn, and all are safe: 7 fail the reads that record the labels, before deleteAll(); 226 fail while the new segment is written, after which the writer refuses further use and the last commit survives; the last 36 come at the end of the final commit, after every label had been re-added, so the ordinals are intact whichever commit is on disk. All 5 appender faults break the ordinals. A field the appender adds that IndexWriter rejects (a term over 32766 bytes) has the same effect as an appender exception: close() commits size 3.

Taxonomy root 0, d 1, d/a 2, d/b 3, d/c 4, committed; main-index documents for a, b, c store 2, 3, 4; counts a=1 b=1 c=1. The appender throws on d/b. After try-with-resources close(): committed size=3, d/b=-1, d/c=-1; FastTaxonomyFacetCounts throws Index 3 out of bounds for length 3. Kept open instead: addCategory(d/w) = 3, addCategory(d/z) = 4; with one w and one z document the counts are a=1 w=2 z=2 (expected a=1 b=1 c=1 w=1 z=1). rollback() instead of close(): size 5, counts unchanged. Same on main with assertions on and off and on 10.5.2 (assertions on).

• Who is exposed. Users of this opt-in, @lucene.experimental class. Its javadoc makes reindexWithNewOrdinalData the required way to update the per-category data (ReindexingEnrichedDirectoryTaxonomyWriter.java:31-34), so every data refresh goes through it. Typical throwing appenders: a lookup that finds no data for a label, a validation error, or the root trap. A read-only code search found only copies of Lucene and one port as users of the class.
• Trigger. An exception from the appender, or a non-aborting rejection of a field it added (Lucene's immense-term check), during the reindex; then close() or continued use instead of rollback(). Deterministic and single-threaded, with the default codec and IndexWriterConfig; one exception is enough (executed with 5 and 7 categories; nothing on the path depends on the size).
• Effect. Durable: the truncated taxonomy is committed and survives reopen. Main-index documents keep ordinals that resolve to nothing (facet counting throws ArrayIndexOutOfBoundsException) or, after continued use, to other labels (silently wrong counts). Repair needs the taxonomy from a backup or a re-index of both indexes (inferred, not run).
• Mitigating. The caller sees the exception first, and rollback() right after it keeps the last commit. Workaround until a fix: commit the taxonomy right before the call and call rollback(), not close(), if it throws.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: expected:<2> but was:<-1>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.junit.Assert.assertEquals(Assert.java:633)
       org.apache.lucene.facet.taxonomy.directory.TestReindexOrdinalDataFailureAtomicity.testFailedReindexThenCloseKeepsOrdinals(TestReindexOrdinalDataFailureAtomicity.java:46)

• Contract. ReindexingEnrichedDirectoryTaxonomyWriter.java:82-86: "Delete the existing taxonomy index and recreate it using new ordinal data. The ordinals themselves will be preserved, so the caller does not need to update references to them in the main index." The class javadoc (:31-34) says that to update the custom data "it is required to" call this method. The deleteAll javadoc (DirectoryTaxonomyWriter.java:893-898) names it as the example of regenerating the taxonomy "taking care that ordinals are indexed in the same order as before". TaxonomyWriter.addCategory returns the existing ordinal of a category already present (TaxonomyWriter.java:53), yet after the failure a lost category's ordinal goes to a new one.
• Lucene's own conventions. IndexWriter.addDocument: "if an Exception is hit (for example disk full) then the index will be consistent, but this document may not have been added" (IndexWriter.java:1465-1466). IndexWriter.addIndexes: "This method is transactional in how Exceptions are handled ... either no indexes will have been added or they all will have been" (:3084-3086). The addDocument promise covers one document, not a sequence of calls like this method's; the point is that Lucene states what a failure leaves behind, and this method does not.
• Tests. The only upstream test of the method, TestOrdinalData.testReindex (:169-181), covers the success path. Its appender guards the root (:65-67) and its tearDown closes the writer with IOUtils.close(taxoWriter) (:121), the idiom that commits the damage. No test covers a failing appender.
• History. The class came with #12337 "Index arbitrary fields in taxonomy docs" (stefanvodita, for #12336, merged 2024-02-08). Its review covered the API shape, updatable doc values, the deleteAll rename and its javadoc warning, and @lucene.experimental; nobody discussed a failure inside the re-add loop, rollback() or what close() commits afterwards. The 2023-05 dev@ design thread does not either.
• Legal trigger. Public API only; the appender is the caller's BiConsumer and no javadoc restricts it. The root trap and Lucene's immense-term check fire without the caller throwing on purpose.

Strongest case (a cross-model refute review, a cross-model intended-lens review that rated it strong but not decisive, and the devil's-advocate check): "the ordinals will be preserved" is a postcondition of a normal return, and the method says it will "Delete the existing taxonomy index"; the caller's own callback threw; rollback() is documented (DirectoryTaxonomyWriter.java:855-865; IndexWriter.deleteAll "can be rolled back using rollback()", IndexWriter.java:2731-2733) and works here, while close() is documented to commit (DirectoryTaxonomyWriter.java:283-286); a maintainer (s1monw, 2019, on open #9738 / LUCENE-8692) said IndexWriter failures from deleteAll(), addIndexes() and commit paths "might cause documents go missing etc. but we treated them not as fatal or tragic events since a user could always call rollback()"; and the class is @lucene.experimental.

Why it fails: nothing documents, for this method, a pending delete after a failure or a rollback() requirement; the method performs the destructive deleteAll() and the final commit() itself, so it presents as one self-contained operation whose stated purpose is exactly what breaks. #9738 concerns IndexWriter's low-level failures, is still open, and in it the same maintainer calls that handling inconsistent; for a non-aborting per-document failure, the kind that triggers this bug, Lucene documents consistency without a rollback, though only for that one document. The usual close idioms commit the damage, and DirectoryTaxonomyWriter has no way to close without committing. @lucene.experimental was asked for in #12337 "so we reserve the right to break them" (the API), not to accept silent data loss. Verdict: a defect under the documented API. Residual risk: maintainers may prefer a javadoc note ("call rollback() after a failed reindex") to a code fix.

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

One file, ReindexingEnrichedDirectoryTaxonomyWriter.java (+24/-6): commit() before recording the paths, so a rollback restores exactly the taxonomy the call started from; deleteAll(), the re-add loop and the final commit() inside try; on any Throwable from that block, rollback() (which keeps the last commit and closes the writer), rollback failures added as suppressed, and rethrow. A javadoc paragraph says so. The snippet is condensed (comments and the javadoc left out; getDirectory() inlined).

ensureOpen();
commit(); // first, so a rollback restores exactly this taxonomy
this.ordinalDataAppender = ordinalDataAppender;
List<FacetLabel> ordinalToPath = recordPathsInOrder(getDirectory());
try {
  deleteAll();
  for (FacetLabel categoryPath : ordinalToPath) {
    addCategory(categoryPath);
  }
  commit();
} catch (Throwable t) {
  try { rollback(); } catch (Throwable t2) { t.addSuppressed(t2); }
  throw t;
}

Trade-offs

• An extra commit at the start when changes are pending. It is allowed by the taxonomy-first commit order (the method commits at the end anyway), and it also makes the recorded paths include categories added since the last commit, which removes the trigger of a related success-path bug in the same method.
• Scope. The try starts at deleteAll(). A failure before it (the first commit(), or reading the labels) is rethrown without a rollback: nothing has been deleted, but after a failed read the writer stays open with the new appender installed. If rollback() itself throws (not tested), it is added as suppressed, and the taxonomy writer may then not be marked closed, because DirectoryTaxonomyWriter.rollback() closes only after indexWriter.rollback() returns (DirectoryTaxonomyWriter.java:863-864); nothing is committed on that path.
• The writer is closed after a failure in the try. Further calls throw AlreadyClosedException and close() is a no-op (DirectoryTaxonomyWriter.java:289), but a caller's own rollback() also throws AlreadyClosedException, because DirectoryTaxonomyWriter.rollback() starts with ensureOpen() (:862), unlike IndexWriter.rollback() (IndexWriter.java:2577). Executed: the root-trap test's rollback() control fails that way with the fix. A PR could also let rollback() return on an already-closed writer (not built).
• Keeping the writer open after a failure is not possible this way, since the internal IndexWriter's rollback closes it.
• Taxonomies already damaged are not repaired. The file is identical from 9.10.0 to main, so the patch backports unchanged.

What the checks validated

• Scratch worktree at 645d4fe14a20, gradle, JDK 25: the new test fails on stock and passes with the fix, assertions on and off.
• The whole facet module with the fix: 40 suites, 307 tests (306 upstream + the new one), 0 failures, 8 skipped, including TestOrdinalData and TestDirectoryTaxonomyWriter; :lucene:facet:check -x test (formatting, ecjLint, forbiddenApis and the other static checks) passes.
• Standalone runner with the patched class shadowed: the new test passes with assertions on and off, and the root-trap test now commits size 4. Earlier, the same logic passed the finder's 2 tests, 44 upstream facet tests, and the failure-point enumeration with 0 violations.
• After the fact checks the javadoc paragraph was reworded (code unchanged) and re-validated the same way: formatting and static checks, the new test with assertions on and off, the whole facet module (40 suites, 307 tests, 0 failures, 7 skipped with another seed) and two standalone shadow runs.

Call the appender on a throwaway Document for each recorded label before deleteAll().

Trade-offs

An appender exception (the repro, the root trap) then leaves the taxonomy and the writer untouched and open, but the appender runs twice per label, and failures that are not the appender's, such as IndexWriter rejecting an appended field, still need option A's rollback.

Document that if the method throws once it has started deleting the taxonomy (any failure from deleteAll() on, such as an appender exception), the taxonomy has been deleted in the writer and only partly re-added, and the caller must call rollback(), not close(); a failure before that (ensureOpen(), reading the labels) leaves the taxonomy untouched.

Trade-offs

No code risk, and rollback() already restores the last commit, but try-with-resources and IOUtils.close still commit the damage, and the method's promise stays unconditional.

For example with replaceTaxonomy; the writer stays open and untouched until the swap.

Trade-offs

Needs a temporary directory and a second write of the whole taxonomy, and replaceTaxonomy has the same failure-atomicity problem.

Method: an executed failure-injection probe (non-formal tier: the first signal was an executed test, not a model). A planning probe for exception safety in the taxonomy writers made the appender fail on its third call during reindexWithNewOrdinalData; after close() the committed taxonomy had 2 categories and d/a, d/b, d/c resolved to -1, on main, 10.5.2, 9.12.3 and a 10.6 release candidate (2026-10-28).

• The hunt reduced it to a public-API JUnit on the stock jars (2 of 2 fail with assertions on and off; 20 of 20 over 10 iterations).
• It then ran a bounded-exhaustive single-fault enumeration on the stock jar: each of the 269 directory calls of a successful reindex failed in turn (all safe: 7 fail before deleteAll(), 226 make the writer refuse further use with the last commit intact, and 36 come at the end of the final commit, after every label was re-added), and the appender failed at each of its 5 calls (all break the ordinals), each under two follow-up probes (close() only; one more category, then close()). With the fix sketch: 0 violations.
• The same unit found the sibling pattern in DirectoryTaxonomyWriter.replaceTaxonomy, filed separately.