AnalyzingInfixSuggester: a build() that throws after its commit leaves refresh() inert, so the old suggestions stay served and the next add() silently switches to the failed build

AnalyzingInfixSuggester.build() (and BlendedInfixSuggester, which inherits it) commits the new suggestions with default settings (closeIndexWriterOnBuild=true) and only then opens a SearcherManager on them. If that open throws (or commit() throws after its commit point is durable), the catch block rolls the writer back (which cannot undo a commit), sets writer = null and keeps the old SearcherManager. refresh() is a no-op while writer == null, so lookups keep serving the old suggestions while the directory holds the build that threw, and the next add()/update(), or reopening the directory, silently switches to it. A second form needs no I/O fault: build() first closes, and so commits, a writer holding an unrefreshed add()/update(), and if the rebuild then fails on its input, refresh() never shows that change. Until 9.1.0 the same failure was loud (suggester was not built); LUCENE-10292 made it silent in 9.2.0. Every release from 9.2.0 to 10.5.2 and main is affected. Fix: after the rollback, reopen the served SearcherManager from the directory's latest commit.

• Writer: the IndexWriter the suggester writes suggestions with. build() opens a new one in CREATE mode; add()/update() open one in APPEND mode on the latest commit when none is open (ensureOpen(), AnalyzingInfixSuggester.java:418-434).
• Commit: what is durable in the directory, and what a new suggester on the same directory loads (:287-290). IndexWriter.close() also commits (commitOnClose defaults to true).
• SearcherManager: what lookups read. refresh() reopens it, but only while a writer is open (:489-498).
• closeIndexWriterOnBuild (default true, :163) and commitOnBuild (default false): either one makes build() commit (:358-359).

1. build() closes any open previous writer (:330-333), which commits any add()/update() not yet refreshed.
2. It indexes the input with a new CREATE writer, commits (:358-359, default settings), and only then calls setAndCloseOldSearcherManager(new SearcherManager(writer, null)) (:361). Until that line the old SearcherManager is still installed: LUCENE-10292 keeps it so that lookups keep working during a rebuild.
3. If anything throws after a commit (an IOException while the new reader opens its files, or commit() failing after the new commit point is durable), the catch block (:362-368) runs writer.rollback(), which returns the index to the last commit, i.e. the one just made; sets writer = null; and rethrows. On these paths the old SearcherManager stays. (The hunt's model also found a split in the opposite direction, with commitOnBuild=false and closeIndexWriterOnBuild=false: closing the old SearcherManager throws after the new one is installed, :298-300, so the rolled-back, never-committed build is served while the directory keeps the previous build; model only.)
4. refresh() then does nothing: it refreshes only if (writer != null), with the comment "writer was committed and closed after the index was built and before searchMgr was constructed, so refresh is unnecessary" (:496-497), which is false on this path.
5. The next add()/update() runs ensureOpen(), which opens an APPEND writer on the latest commit (the failed build) and installs a new SearcherManager on it (:431), so lookups switch to the failed build at that call, even before refresh(). Reopening the directory loads the same commit.

Second form (no I/O fault). After build([old apple]), an add("added apricot") that is not yet refreshed, then a rebuild whose input fails: an InputIterator error, or one entry over IndexWriter's 32766-byte term limit, which addDocument rejects with IllegalArgumentException. Step 1 has already committed the addition; the rebuild's own writer is rolled back; refresh() cannot show the committed addition: served [old apple], stored [added apricot, old apple].

What is stored is complete. After the first form the directory holds the complete dictionary build() was asked to produce: every entry was indexed and committed before the failure. No corruption was observed in the exercised cases (reopened indexes were readable; the hunt's safety probe passed CheckIndex). The defect is that build() reports failure while its result is already durable, refresh() cannot publish it, and an unrelated call or a restart publishes it later. A later successful build() restores consistency.

BlendedInfixSuggester extends AnalyzingInfixSuggester without overriding build() (BlendedInfixSuggester.java:56) and behaves the same (executed in the finder's test).

• Who is exposed. AnalyzingInfixSuggester and BlendedInfixSuggester users (opt-in suggesters, @lucene.experimental, AnalyzingInfixSuggester.java:95) who call build() again and keep using the instance after it throws, which is the natural reaction (log the error, keep serving). The default constructor suffices (closeIndexWriterOnBuild=true); commitOnBuild=true behaves the same (executed: the independent reproducer's commitOnBuild=true test and the version probe's commitOnBuild=true scenario). Dictionary size does not matter: the repro's builds have one or two entries.
• First form (main). An IOException while the new reader opens its segment files right after the commit (for example a transient storage error or an open-file or memory-map limit; only synthetic faults were run), or commit() failing after the commit point is durable. The served suggestions stay old until the next add()/update() or a restart, then silently become the failed build's.
• Second form (secondary). An add()/update() not yet refreshed, then a rebuild that fails on its input (one entry over 32766 bytes is enough). The change is committed but invisible after refresh() until the next add()/update() or a reopen. It is weaker, because a successful rebuild would have dropped that change anyway.
• No error at lookup time. Only build() throws; refresh() and lookup() succeed, and nothing tells the caller that the served view and the directory differ.
• Deterministic once triggered: the finder's JUnit failed 80 of 80 runs over 20 seeds; the repro fails the same with assertions on and off and in 10 of 10 iterations with a random seed.
• Downstream, by code reading only. Solr's AnalyzingInfixLookupFactory and BlendedInfixLookupFactory pass commitOnBuild=true, and its SuggestComponent's buildOnCommit/buildOnStartup listener catches a build exception, logs it and keeps the same lookup in service (a suggest.build=true request gets the exception back instead); Solr calls only build(), so it can reach the first form (stale until the next successful build or a core reload, which loads the failed build), not the second. No use was found in Elasticsearch or OpenSearch.
• How likely. Rare: every default rebuild is exposed, but only an exception in that narrow window, or a rebuild failing right after an unrefreshed change, triggers it. No user report since 9.2.0 (2022) was found.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: expected:<[another apricot, old apple]> but was:<[another apricot, new avocado]>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:120)
       org.junit.Assert.assertEquals(Assert.java:146)
       org.apache.lucene.search.suggest.analyzing.TestInfixFailedBuildAfterCommit.testAddAfterFailedBuild(TestInfixFailedBuildAfterCommit.java:67)

• The code's own failure handling. build()'s catch (AnalyzingInfixSuggester.java:362-368) runs writer.rollback(); writer = null; and rethrows, so it treats the build as failed. A rollback discards only uncommitted work (IndexWriter.java:2566-2574), so the commit made a few lines earlier survives and nothing brings the served view in line with it.
• refresh()'s comment states the broken invariant (:496-497): "writer was committed and closed after the index was built and before searchMgr was constructed, so refresh is unnecessary".
• The javadoc. refresh(): "Reopens the underlying searcher" (:486). add(): "Adds a new suggestion ... After adding or updating a batch of new suggestions, you must call refresh in the end in order to see the suggestions in lookup" (:436-439); update() the same (:447-451). Here add() + refresh() replaces every served suggestion, and in the second form a committed add() is never shown by refresh().
• History. Up to 9.1.0 build() closed and nulled the SearcherManager first, so a failed build left refresh() and lookup() throwing IllegalStateException: suggester was not built (executed on 9.1.0, 9.0.0, 8.11.4, 6.4.0). LUCENE-10292 (9.2.0; CHANGES: "Fix AnalyzingInfixSuggester / BlendedInfixSuggester to correctly return existing lookup() results during concurrent build()") kept the old manager and left the failure path unchanged; nobody in that issue discusses a build() that throws. The record gives no sign that the silent state was designed.
• Tests. No upstream test in lucene/suggest injects faults into the suggester or asserts the state after a failed build().
• The oracle holds either way. The repro asserts only that, after refresh(), the instance serves what its directory holds and that add() adds one suggestion. "The failed build is applied" and "it is not applied" both pass; only the split fails, and a control in which the failed build() writes no commit passes.

The strongest works-as-designed case (intended-behaviour checker, the cross-model refute lens, the adjudication): Lookup.build promises nothing about failures ("Builds up a new internal Lookup representation", Lookup.java:178-182); the class is @lucene.experimental; the commitOnBuild javadoc says "Call commit after the index has finished building. This would persist the suggester index to disk" (:192-194); and in LUCENE-10292 a reviewer said rebuild behaviour "isn't guaranteed or documented by the Lookup class", while its author said "I didn't set out to make any hard & fast guarantee about the thread safety of all lookups".

This doubt is real: nothing documents that the instance stays usable after build() throws, so the post-failure semantics may be debated. It does not settle the case: those LUCENE-10292 remarks are about concurrent lookup() during build(), not about failures. Silence about failure semantics does not license a state in which refresh() is permanently inert and an unrelated add() swaps the dictionary; the code's own rollback and refresh() comment point the other way; and even if a finished build may stay durable, the repro still fails, because the instance does not serve it. @lucene.experimental covers API stability, not wrong results. The refute lens returned "uncertain", high confidence in the facts, "the only remaining doubt is contractual"; every other check returned "real".

Option A was applied in a scratch worktree at a9779ef3c193 and run through gradle (seed DEADBEEF); option B was checked against the repros with its class shadowed onto the main jars; C and D were not run.

In build()'s catch block (AnalyzingInfixSuggester.java:362-368), after writer.rollback(); writer = null;, install new SearcherManager(dir, null) on the latest commit when an index exists, suppressing a second failure into the original exception. Whichever step failed (the build's own commit, a commit that threw after it was durable, or the previous writer's commit on close), the latest commit is exactly what is durable, so serving it restores the invariant refresh() relies on, and the next add() adds to what is served. build() still rethrows.

           writer = null;
         }
+        // The rollback cannot undo what is already durable: the previous writer was closed (and so
+        // committed) above, and this build may have been committed before the failure. Serve what
+        // the directory now holds; otherwise refresh(), a no-op once writer == null, can never
+        // bring the served suggestions back in line with the index.
+        try {
+          if (DirectoryReader.indexExists(dir)) {
+            setAndCloseOldSearcherManager(new SearcherManager(dir, null));
+          }
+        } catch (Throwable t2) {
+          t.addSuppressed(t2);
+        }
         throw t;

Trade-offs

• A policy choice, not a transactional rollback. After the first form the instance serves the build that threw (the complete requested dictionary), as a restart would. Maintainers may prefer other semantics (option B plus a fallback, a failed-state flag, or documentation).
• Not covered: an exception from the writer.close() before the try (:331) or after a successful build (:370-373), a second failure inside rollback(), and a recovery open that fails too (then the old view stays, as today). The recovery catches any Throwable and adds it to the original exception as suppressed (untested for VM errors); narrowing it to IOException is a reasonable variant.
• Cost: one extra directory reader open, only on the failure path. No format or API change.

What the checks validated

• Stock main a9779ef3c193 (gradle, seed DEADBEEF): the repro fails 2 of 3 with assertions on and off; TestAnalyzingInfixSuggester and TestBlendedInfixSuggester pass (23 + 9 tests).
• With the fix: the repro passes 3 of 3 with assertions on and off; TestAnalyzingInfixSuggester plus TestBlendedInfixSuggester pass (32 tests); all of :lucene:suggest:test passes with the repro class added (32 classes, 239 tests, 0 failures, 7 skipped); ecjLintMain, ecjLintTest and checkGoogleJavaFormat pass for suggest; fix.diff applies cleanly to a clean tree.
• With the fix shadowed onto the main jars: the finder's JUnit 4 of 4 and the independent reproducer's 9 of 9 (including commit() throwing after the commit is durable) pass.
• Earlier, in the hunt, the same change: 0 violations in the length-2 model run (1,952 without it), and the over-long-entry test and the downstream probe pass.

Open new SearcherManager(writer, null) first, then commit, closing the new manager if the commit throws. A failure to open the reader then happens before anything is durable, so the build is cleanly "not applied".

Trade-offs

Covers only the reader-open case. Executed with the class shadowed onto the main jars: the second form still fails in the repro (served [old apple] vs stored [added apricot, old apple]), in the finder's JUnit (1 of 4) and in the independent reproducer's JUnit, which also still fails its commit-throws-after-durable test (2 of 9). It can be combined with option A, which then handles the rest.

What the checks validated

Repros only (classpath runner, seed DEADBEEF); the module suite was not run with it.

When no writer is open, let refresh() reopen the served searcher from the latest commit.

Trade-offs

Not run. Changes the success path: every refresh() after a successful build with closeIndexWriterOnBuild=true would check the directory, where today it costs nothing.

A javadoc note on build().

Trade-offs

Not run, not recommended: leaves the silent switch in place for everyone who does not read it.

Method: operation-level model-based testing on the stock jar (semi-formal tier), in a hunt unit on the infix suggester's lifecycle.

• The harness ran scripts of build(), add(), update(), refresh(), commit() and reopen calls against an exact reference model of the served, pending and committed suggestion sets and the writer state, over the commitOnBuild x closeIndexWriterOnBuild configurations. A failed call had to be explainable as applied, not applied, or served = committed.
• It injected a MockDirectoryWrapper.Failure at every create, open, sync, rename, delete and close point of every call (the default boundary mode; buffered writes excluded, and no recorded run used the all-points mode), one per run, bounded-exhaustively: length 1 (36 scripts, 2,520 fault runs, 108 violations, all this class), length 2 (324 script-config pairs, 69,760 fault runs, 1,952 violations, all with the fault in build(); 0 with the fix shadowed in) and length 3 on separate build machines (2,916 script-config pairs, 1,104,615 fault runs).
• The hypothesis (a rollback after a commit) came from reading the code while the model was written; the model then flagged it independently. The second form came from triaging a state the model tolerated and was confirmed with a JUnit.
• Found 2026-10-26 on main a9779ef3c193; then checked by an independent reproducer, an intended-behaviour devil's advocate, reviews by a model from another family (OpenAI GPT-5.6), an adjudication, a history, prior-report and security follow-up, and a credibility audit.