DirectoryTaxonomyWriter gives an existing category a second ordinal after an IOException in addCategory

DirectoryTaxonomyWriter keeps a label-to-ordinal cache (LruTaxonomyWriterCache(4000) by default since 9.5.0) and a flag, cacheIsComplete, that lets it treat a cache miss as a new label. When a put evicts entries, addToCache first refreshes the writer's near-real-time reader and only then clears the flag. If that refresh throws an IOException, for example because a segment file cannot be opened, the flag stays set although a third of the cache was just evicted, and IndexWriter stays open. The next addCategory of an evicted label indexes it again under a new ordinal, and commit() persists both: dim/v0 at ordinals 2 and 4001, facet count 1 instead of 2, CheckIndex clean. A second path: after an existing taxonomy is read into the cache, addCategoryDocument indexes a new category before it opens a reader for the parent array; if that open throws, a retry gives the category a second ordinal (13 and 14). Every release from 3.4.0 (with an evicting LRU cache) to 10.5.2 and main is affected; with the default writer from 4.1.0 (second path) and 9.5.0 (first path).

• Taxonomy and ordinal. Taxonomy facets store each label, such as dim/v0, once in a side index, as one document; its document number is the label's ordinal. Documents in the main index store ordinals, and facet counts are kept per ordinal. TaxonomyWriter.addCategory must return the existing ordinal of a label that is already there.
• Cache and the "complete" flag. DirectoryTaxonomyWriter caches label-to-ordinal entries. The default LruTaxonomyWriterCache(4000) evicts: when a put makes it exceed 4,000 entries, it drops the least recently used ones down to 2,666, so the 4,001st entry evicts 1,335. While the flag cacheIsComplete is set, a cache miss means "new label" and nothing is looked up on disk.
• The writer's reader. To answer a miss when the cache is not complete, the writer looks the label up through a near-real-time reader of its own index. After an eviction it must refresh that reader, because evicted labels may have been added after the reader was opened.

1. addToCache (DirectoryTaxonomyWriter.java:484-494): cache.put reports an eviction; the writer calls refreshReaderManager() (:491) and only then sets cacheIsComplete = false (:492).
2. The refresh reaches IndexWriter.getReader, which flushes and then opens the new segments. If opening a file throws an IOException, getReader rethrows it without closing IndexWriter (it treats only an Error as tragic, IndexWriter.java:711-721). The flag stays set and the reader stays old.
3. The next addCategory(dim/v0) misses in the cache; findCategory returns the miss at once because the flag is set (:327), and the label is indexed again as ordinal 4001.
4. Clearing the flag first is not enough: the on-disk lookup and the cache fill (perhapsFillCache) would then use the old reader, which does not see the evicted labels either. Nothing on the addCategory path refreshes it until the next eviction.

1. A reopened taxonomy reads all its labels into the cache after 11 misses; if they all fit, it sets the flag and closes its reader (:652-662).
2. The next new label is indexed (:468) before the writer re-opens a reader for the parent array (getTaxoArrays(), :475); the cache update comes last on purpose (:477-479, LUCENE-4596). If that open throws, the label is in the index but not in a cache that is still marked complete, and a retry indexes it again: new at 13 and 14.

Default writer and FacetsConfig: 3,998 values fill the cache (root, dim and 3,998 values); the 3,999th document's refresh fails and the document is skipped; one more dim/v0 document is indexed. Committed taxonomy: 4,002 ordinals, dim/v0 at 2 and 4001; FastTaxonomyFacetCounts.getSpecificValue("dim", "v0") = 1 instead of 2. Writer view: addCategory("l0") returns 4001 instead of 1. Site B: the retry returns 14 instead of 13. Same with assertions on and off, on main and 10.5.2.

• Who is exposed. Users of the opt-in taxonomy facet module with the default writer constructor (and the IndexWriterConfig it builds itself), cache and codec, on any Directory, who continue after a failed addCategory or FacetsConfig.build, as ordinary per-document error handling does: the wrapped IndexWriter stays open, and no javadoc asks for a rollback (none explicitly promises reuse either). A read-only code search of the default branches of Elasticsearch, OpenSearch and Solr found no use of DirectoryTaxonomyWriter; custom applications that embed the module are the ones exposed.
• What triggers it. A non-Error IOException exactly while the writer opens its reader inside addCategory: the refresh after an eviction (with the default cache about once per 1,335 new categories once it is full; a probe saw 13 reader opens in 20,000 addCategory calls), or the first reader open after an existing taxonomy that fits in the cache was read into it. Realistic causes: a failed memory map (MMapDirectory reports it as a plain IOException) or running out of file handles. A failure in the flush itself closes IndexWriter instead; every repro injects the failure in the reader open after a successful flush. Labels alone never cause it (0 duplicates in 80,000 adds and re-adds).
• Effect. One label, two category documents, persisted by commit(); CheckIndex reports the taxonomy clean. Rolling back both writers before the next commit discards it (with every other uncommitted change); after a commit it takes a clean backup or a rebuild. Counts for the label are split, getAllChildren lists it twice, and when the evicted label is a dimension, getAllDims lists the dimension twice. Drill-down matching stays correct (it uses path terms): 0 mismatches over 3,998 labels. Merging the damaged taxonomy with addTaxonomy after a one-segment merge mapped the second copy to ordinal 0, or threw EOFException with DiskOrdinalMap.
• Scale. None needed: the first eviction comes at the 4,001st cache entry.
• Not a separate bug. The failed call's own document is also persisted, which matches IndexWriter's "this document may not have been added" wording. In the end-user scenario FacetsConfig.build throws before the main index's addDocument, so the taxonomy then holds a category (dim/v3998) that no main-index document uses.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: ordinal returned for existing category l1 expected:<1> but was:<4001>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.apache.lucene.facet.taxonomy.directory.TestTaxonomyWriterFailedRefreshDuplicatesOrdinal.testExistingCategoryKeepsOrdinalAfterFailedEvictionRefresh(TestTaxonomyWriterFailedRefreshDuplicatesOrdinal.java:58)

• Contract. TaxonomyWriter.java:51-53: "If the category was already present in the taxonomy, its existing ordinal is returned." TaxonomyWriterCache.java:30-32 (the same rule in other words at :55-58): the caller "can only infer that the category doesn't exist if it knows the cache to be complete (because all the categories were loaded into the cache, and since then no put() returned true)". The writer's own comments: the cache "becomes incomplete ... if a put() to the cache ever returned true" (DirectoryTaxonomyWriter.java:110-117); after an eviction "we must now refresh the reader" (:486-490); "when this method returns, we must know that the reader manager's state is current" (:497-499); "each term has exactly one document" (:366, :628-629).
• The wrapped IndexWriter keeps working after non-fatal exceptions. It closes itself only "if you hit an Error, or disaster strikes during a checkpoint" (IndexWriter.java:162-165); after an exception in addDocument "the index will be consistent, but this document may not have been added" (:1465-1466). Nothing says the taxonomy writer must be discarded after an exception (nor, explicitly, that it may be reused: continued use is implicit); the rollback() javadoc only describes rollback (DirectoryTaxonomyWriter.java:855-859).
• Site A is the clean case. The label that gets the second ordinal (dim/v0, l0) was added successfully long before; the failed call was for a different label.
• Tests. No facet test injects I/O failures into the taxonomy writer, so the exception path is untested, not waived.
• Precedent. Upstream fixed "an existing category gets a second ordinal" as a bug three times; in none did the duplicate come from a failed addCategory call: LUCENE-4061 (#5133, 2012; a statement-order bug skipped the refresh; shaie: "NoOpCache in this case simulates the case where e.g. LruTaxoWriterCache just evicts entries from it and we fail to refresh the reader, so this is a true bug"), LUCENE-4633 (#5698, replaceTaxonomy left the reader stale) and LUCENE-5048 (#6112, a huge label re-added with a different ordinal; it was reported as a NegativeArraySizeException from the old cache inside addCategory, but the second ordinal came from a short cast on the label length, not from the exception).

Strongest case (the devil's advocate; the cross-model intended-lens reviewer rated it moderate): both classes are @lucene.experimental (TaxonomyWriter.java:47, DirectoryTaxonomyWriter.java:76); no exception-safety guarantee is documented; rollback() exists and TwoPhaseCommit frames it as the recovery tool; IndexWriter itself says a document "may not have been added" after an exception; the facet module never had failure-injection tests; the trigger window is narrow.

Why it fails: @lucene.experimental permits API changes, not duplicate ordinals; the addCategory contract and the cache rule are unconditional, and the code breaks its own stated invariants; "roll back after any IOException" is documented nowhere, is stricter than the IndexWriter the writer wraps (which stays usable after such exceptions), and would discard every uncommitted category for a failure IndexWriter survives; the damage is silent and persisted; upstream treated the same symptom as a bug before. In site A the duplicated label was added successfully long before the failed call, so "the failed call may not have added its document" does not cover it. A rare trigger lowers severity, not intent. For site B the rebuttal is weaker: the retried label is the one whose call threw, and a maintainer could reasonably prefer fail-stop semantics (option D); there it rests on the missing documentation, not on a documented reuse guarantee. Verdict: a defect. Expect the reply "roll back after an IOException"; answer with the contract, the cache rule and the IndexWriter exception contract.

Option A was built and validated with gradle in a scratch worktree; option B was run and is not enough; options C to E were not run in Java. Do not revert LUCENE-4596's order (the cache entry must stay last).

One file, DirectoryTaxonomyWriter.java. A new volatile field readerMayMissCategories. addToCache sets cacheIsComplete = false and the new flag before refreshReaderManager(), and clears the new flag once the refresh returns. addCategoryDocument wraps the parent-array update and addToCache (order unchanged) and, if either throws, clears cacheIsComplete, sets the new flag and rethrows. findCategory, after counting the miss, refreshes the reader when the new flag is set, before perhapsFillCache() and the on-disk lookup.

// addToCache, after cache.put(...) evicted entries
cacheIsComplete = false; // before the refresh, which may throw
readerMayMissCategories = true;
refreshReaderManager();
readerMayMissCategories = false;

// addCategoryDocument: catch (Throwable t) after the document is indexed
cacheIsComplete = false; readerMayMissCategories = true; throw t;

// findCategory, before perhapsFillCache() and the on-disk lookup
if (readerMayMissCategories) {
  refreshReaderManager(); // no-op when no reader manager is open
  readerMayMissCategories = false;
}

Trade-offs

• Keeps LUCENE-4596's order: the cache entry is still published last, so a lock-free cache hit never sees an ordinal whose parent is not stored yet.
• A retry of the failed label finds its already indexed document and returns that ordinal (13); the failed call's document stays, as IndexWriter's wording allows.
• Cost: one extra reader refresh on the first cache miss after a failure; on the normal path a volatile read per cache miss and two writes per eviction. Not benchmarked.
• Taxonomies that already hold a duplicate are not repaired; they need a rebuild.
• DirectoryTaxonomyWriter.java is byte-identical from 10.0.0 to main; the 9.x files differ but have the same two orders, so a 9.x backport needs its own patch. Only main was built with gradle.
• Scope: it covers failures after indexWriter.addDocument succeeded (the reader opens), not an exception from addDocument itself, which is a different root cause tracked separately.

What the checks validated

• Scratch worktree at a9779ef3c193, gradle, JDK 25: the new test fails 4 of 5 on stock and passes 5 of 5 with the fix, assertions on and off; with TestDirectoryTaxonomyWriter: 20 tests, 0 failures.
• The whole facet module: 40 suites, 311 tests (306 upstream + 5 new), 0 failures, with assertions on (7 skipped) and off (8 skipped); :lucene:facet:check -x test (google-java-format, ecjLint, forbiddenApis) passes.
• Standalone runner with the patched class: the new test, the finder's 3 tests, the independent reproducer's tests and control, and the end-user test and control all pass, assertions on and off (the second mode of the finder's, the independent reproduction's and the intended-behaviour check's suites was run by the fact check). In the finder's bounded TLA+ model the same logic has no violation in any of 96 configurations: the 84 with one injected failure check the six observable invariants (no duplicate label, sizes, add results, parents, cache soundness), the 12 failure-free ones also the protocol invariant LookupComplete, which the fix deliberately relaxes after a failure. The model abstracts the reader as a prefix of the indexed documents: corroboration, not a proof.

The obvious two-line swap in addToCache.

Trade-offs

The lookup and the cache fill then use the reader whose refresh failed, which does not see the evicted labels; site B is unchanged. The finder's model shows the same, and a first sketch that refreshed only before the on-disk lookup failed the finder's stale-fill test (testEvictionRefreshFailureThenFillFromStaleReader, "expected:<1> but was:<4>").

What the checks validated

Gradle on main: all 4 failing tests still fail with the same values.

Refresh whenever shouldRefreshReaderManager is set, not only after a failure.

Trade-offs

Correct in the finder's model (variant fixFull), but it reopens the near-real-time reader on cache misses after any new label. Not run in Java.

Treat an exception after indexWriter.addDocument, or a failed eviction refresh, as fatal: roll back and close, so later calls throw AlreadyClosedException and nothing is committed.

Trade-offs

Loses every uncommitted label and forces the application to reopen both writers; stricter than IndexWriter, which survives these exceptions. Not run.

Javadoc only.

Trade-offs

No code risk, but nothing stops the silent corruption, and rollback discards uncommitted labels.

Method: TLA+ model checking (TLC 1.8.0) (semi-formal tier: a hypothesis formed while modelling, then witnessed by the tool), in a hunt unit on concurrent adds to the taxonomy writer.

• The model covers addCategory, the LRU cache with its exact eviction rule, the lazily opened reader, cacheIsComplete, perhapsFillCache and the parent array, with optional I/O failure points. Without failures, every invariant holds in all 96 bounded configurations. The model is bounded and abstracts the reader as a prefix of the indexed documents, so it corroborates the Java finding rather than proving anything about the Java code.
• With one injected failure, the protocol invariant LookupComplete fails in 37 of 48 configurations; minimal traces: a refresh failure after an eviction (18 states) and a reader re-open failure after a complete cache fill (9 states). Clearing the flag first is not enough (NoDuplicateLabel fails), and the stale-reader cache fill came from the model too.
• The traces were reduced to a same-package JUnit on the stock jars, then checked by an independent reproducer, a devil's advocate, a reviewer from another model family (OpenAI GPT-5.6, reading only), an adjudication and an audit.