updateDocument racing commit() can leave two live documents for one key in the commit

Several threads replace documents by key with IndexWriter.updateDocument while another thread calls commit(). DocumentsWriterFlushControl.nextPendingFlush checks "is a full flush running?" under its lock, releases the lock, and only then checks out a flush-pending in-memory buffer (DWPT) without checking again. If the commit's full flush starts in that gap, the indexing thread flushes a buffer of the new, post-commit delete queue inside the commit. The commit point then holds operations above the sequence number commit() returned and, when the newer update's delete misses the older document, two live documents for one key; the duplicate stays until the key is updated again, and a crash after that commit would recover it. With assertions on, the same race shows either as the duplicate or as a tripped doFlush assertion that closes the writer. One public-API test reproduces it on main when the indexing threads outnumber the CPUs; the code dates from LUCENE-3090 (2011), and 10.6.0 and 10 of the 14 releases run from 5.3.0 to 10.5.2 fail too. A one-hunk re-check of fullFlush under the lock fixes it.

1. Indexing threads help flush. After every update DocumentsWriter.postUpdate calls maybeFlush (DocumentsWriter.java:400-410, with the default checkPendingFlushOnUpdate), which calls flushControl.nextPendingFlush() (:493-500).
2. The stale check. nextPendingFlush (DocumentsWriterFlushControl.java:441-469) copies fullFlush and numPending into locals under the monitor (:450-451) and releases it (:452). Then if (numPending > 0 && fullFlush == false) { // don't check if we are doing a full flush (:453) scans the pool and returns checkOutForFlush(next) for the first flush-pending DWPT it can lock (:454-459).
3. No second look. checkOutForFlush (:409-424) is synchronized but never reads fullFlush.
4. The full flush starts in the gap. commit() reaches DocumentsWriter.flushAllThreads (DocumentsWriter.java:667-715), whose markForFullFlush sets fullFlush = true and swaps the delete queue under the monitor (DocumentsWriterFlushControl.java:564, :575), then collects the old queue's DWPTs (:580-606).
5. A new-queue DWPT can already be pending. Indexing continues during the commit, and the RAM policy marks the largest non-pending DWPT pending, possibly another thread's idle one (FlushByRamOrCountsPolicy.java:83-90).
6. The sibling path does it right. checkout() reads fullFlush under the monitor and parks such a DWPT in blockedFlushes until finishFullFlush (DocumentsWriterFlushControl.java:253-272, :397-407, :649-664).
7. Result. The stale thread flushes a new-queue DWPT inside the full flush; flushAllThreads waits for it (DocumentsWriter.java:690) and the commit publishes its segment and its frozen new-queue deletes. DocumentsWriter.java:508-515 asserts that only the full flush's queue flushes during it. That assertion is armed only after markForFullFlush returns (DocumentsWriter.java:680-681), so it catches an out-of-turn flush that starts after the collection but not one that starts during it; with -ea the test shows either the assertion or the duplicate.

In the TLA+ model's counterexample, the out-of-turn flush starts while markForFullFlush is still collecting, so its flush ticket is queued ahead of the old queue's tickets. Its delete packet is then published before old-queue segments that flush after it, and a packet applies only to segments published before it, so the newer update's delete never reaches the older copy (FrozenBufferedUpdates.java:460-462). By code reading this also needs the older version's own delete to have been frozen already, by an old-queue flush whose ticket is ahead of the out-of-turn one: every flush freezes its queue's whole delete buffer (DocumentsWriterPerThread.java:429), and an old-queue packet published after the new segment would delete the newer version instead. The model does not model deletes, so it cannot settle this; the executed duplicates are consistent with it. Neither ordering was instrumented at runtime, and one step of the cited trace (state 23) is not a code schedule: a thread marks another buffer pending while its own is pending, which doAfterDocument does not do (DocumentsWriterFlushControl.java:232-238). Observed directly: an observation-only probe tied every failing commit to a new-queue check-out by nextPendingFlush during that commit's full flush (or the one before), and the -ea stack goes through postUpdate -> maybeFlush -> doFlush.

• The hunt's longer test also checks the sequence-number cut: "commit #88 returned seqNo=547238 but holds a live document of thread 4 with seqNo=547246".
• The same workload with the opposite oracle found 18 of 4,746 commits with keys that had no live document (298 keys: a later update's delete visible without its add); 0 of 4,499 with the fix. Not claimed separately.

• Trigger. Concurrent updateDocument on one writer while commit() runs (softUpdateDocument takes the same updateDocuments path by code reading, IndexWriter.java:1938-1947; not run); an indexing thread descheduled inside a two-statement window of nextPendingFlush just as the full flush starts; a buffer of the new delete queue already marked pending by the RAM policy. Default flush-by-RAM policy and codec; any Directory by code reading (every run used ByteBuffersDirectory); no opt-in feature or expert API. IndexWriter is documented as "completely thread safe".
• Effect. Silent wrong results: the commit point holds two live documents for one key until the key is updated again, and a crash after that commit would recover the duplicate (inferred from commit semantics; no crash was simulated); the commit also holds operations above the sequence number commit() returned. With -ea (tests), it can instead show as a tragic AssertionError that closes the writer.
• How often. With 16 indexing threads pinned to 2 or 4 CPUs every 30 s run failed (10 of 10, 5 of 5); unpinned on 32 cores no run failed in 120 s. The repro lowers the RAM buffer to 0.25 MB and uses NoMergePolicy only to raise the flush and commit rate; an independent test failed with the default merge policy and scheduler at 1 MB. No failure has been shown at the default 16 MB buffer, and no field report is known. CPU-oversubscribed indexing with frequent commits raises the rate.
• Not exercised. NRT reopen runs the same full flush and softUpdateDocument the same update path by code reading, but only commit(), updateDocument and ByteBuffersDirectory were run.

The test pins what its writer uses (the default codec, InfoStream.NO_OUTPUT, ByteBuffersDirectory, a plain IndexWriterConfig); the fixed seed fixes the suite-level settings the framework still randomises (locale, time zone, similarity, the global test codec). It stops after at most 5,000 commits. The 0.25 MB RAM buffer and NoMergePolicy only raise the flush and commit rate. No public-API hook exists inside the two-statement window, so the test is reliable only with more runnable indexing threads than CPUs: pinned to 2 CPUs it failed 6 of 6 times with assertions off and 4 of 4 with assertions on (an independent fact check: -ea 1 of 2 standalone runs and its gradle run); unpinned on 32 cores a similar test did not fail in 120 s.

Use --no-daemon with taskset: Gradle then forks a fresh single-use daemon under the pin and the test JVM inherits it, which a daemon started earlier outside the pin would not. In these local, non-CI runs the Lucene test plugin added -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 (TEST_JVM_ARGS replaces them), so the test JVM reported cpus=1 under the 2-CPU pin; the test still failed through gradle. Run ./gradlew tidy before putting the test in a PR.

The hunt's earlier, longer test (supplementary, not needed to file) also checks that no live document's updateDocument sequence number is above the one commit() returned.

Regression test: this same test; it passes with the recommended fix.

cp $REPRO/TestUpdateDocumentConcurrentCommit.java $LUCENE/lucene/core/src/test/org/apache/lucene/index/
cd $LUCENE && taskset -c 0,1 ./gradlew --no-daemon :lucene:core:test --tests TestUpdateDocumentConcurrentCommit -Ptests.seed=DEADBEEF -Ptests.asserts=false
# assertions on: drop -Ptests.asserts=false (the doFlush assertion closes the writer)

# standalone runner, main 645d4fe14a20, taskset 2 CPUs, seed DEADBEEF, assertions off (15.6 s)
testAtMostOneLiveDocumentPerKey  FAILED
  java.lang.AssertionError: commit 106, key 388
    at org.apache.lucene.index.TestUpdateDocumentConcurrentCommit.testAtMostOneLiveDocumentPerKey(TestUpdateDocumentConcurrentCommit.java:77)
# 5 more iterations, assertions off: 5 of 5 fail (commits 123, 136, 367, 429, 826)

# assertions on (88 s): the commit thread's exception, with the indexing thread's AssertionError as its cause
java.lang.IllegalStateException: this writer hit an unrecoverable error; cannot commit
  org.apache.lucene.index.IndexWriter.startCommit(IndexWriter.java:5919)
  org.apache.lucene.index.IndexWriter.prepareCommitInternal(IndexWriter.java:3915)
  org.apache.lucene.index.IndexWriter.commitInternal(IndexWriter.java:4253)
  org.apache.lucene.index.IndexWriter.commit(IndexWriter.java:4215)
Caused by: java.lang.AssertionError: expected: DWDQ: [ generation: 811 ] but was: DWDQ: [ generation: 812 ] true
  org.apache.lucene.index.DocumentsWriter.doFlush(DocumentsWriter.java:515)
  org.apache.lucene.index.DocumentsWriter.maybeFlush(DocumentsWriter.java:496)
  org.apache.lucene.index.DocumentsWriter.postUpdate(DocumentsWriter.java:407)
  org.apache.lucene.index.DocumentsWriter.updateDocuments(DocumentsWriter.java:450)
# 3 more iterations, assertions on: 3 of 3 fail (2 duplicates at commits 1235 and 1257, 1 assertion)

# gradle, scratch worktree of 645d4fe14a20, taskset 2 CPUs, -Ptests.seed=DEADBEEF, assertions on (test 355 s)
java.lang.AssertionError: expected: DWDQ: [ generation: 1259 ] but was: DWDQ: [ generation: 1260 ] true
  at org.apache.lucene.index.DocumentsWriter.doFlush(DocumentsWriter.java:515)
  ... maybeFlush(DocumentsWriter.java:496) <- postUpdate(DocumentsWriter.java:407)
(then, while the writer rolls back: AssertionError: pendingNumDocs 12635 != 11985 totalMaxDoc)
:lucene:core:test (FAILURE): 1 test, 1 failure

# gradle, the same with -Ptests.asserts=false (test 122 s)
java.lang.AssertionError: commit 646, key 262
  at org.apache.lucene.index.TestUpdateDocumentConcurrentCommit.testAtMostOneLiveDocumentPerKey(TestUpdateDocumentConcurrentCommit.java:77)
:lucene:core:test (FAILURE): 1 test, 1 failure
# with the fix, gradle, same worktree and pin: :lucene:core:test (SUCCESS): 1 test, assertions off (882 s) and on (1,267 s)

# with the fix, standalone runner: all 5,000 commits clean, assertions off (329 s) and on (405 s)

# independent fact check (2026-10-30), its own scratch worktree of 645d4fe14a20, same commands, 2 pinned CPUs
#   gradle -Ptests.asserts=false: AssertionError: commit 50, key 6 (about 18 s); with the fix: SUCCESS (1,076 s)
#   gradle, assertions on: the same doFlush:515 AssertionError (225 s)
#   standalone: -da commit 328, key 588; -ea 1 of 2 runs failed (the assertion); with the fix -da and -ea pass

• Contracts. "IndexWriter instances are completely thread safe, meaning multiple threads can call any of its methods, concurrently" (IndexWriter.java:167-168). updateDocument: "The delete and then add are atomic as seen by a reader on the same index (flush may happen only after the add)" (:1902-1904). commit(): "All sequence numbers <= this value will be reflected in the commit, and all others will not" (:4208-4210; the same on prepareCommit, :3730-3732).
• The code's intent. The comment at the check, "don't check if we are doing a full flush" (DocumentsWriterFlushControl.java:453); checkout() parks a pending DWPT during a full flush (:253-272); numBlockedFlushes describes a DWPT that "needs flushing but must not be flushed until the full flush has finished" (:722-726); doFlush asserts that only the full flush's queue flushes during it (DocumentsWriter.java:508-515), and assertTicketQueueModification says "only modifications from the current flushing queue are permitted while doing a full flush" (:654-658). prepareCommitInternal clones the segment infos "so that no partial changes (eg a delete w/o corresponding add from an updateDocument) can sneak into the commit point" (IndexWriter.java:3845-3849). The 4.0 CHANGES entry: flushes that occur while a full flush is running "are queued and will happen after all DWPT involved in the full flush are done flushing" (lucene/CHANGES.txt:15736-15741).
• Tests. TestIndexingSequenceNumbers.testStressConcurrentCommit (@Nightly) checks the same contract with concurrent updates and commits ("this commit point should reflect all operations <= this seqNo"); TestIndexWriter.testNeverCheckOutOnFullFlush asserts that nothing is checked out once a full flush has started through checkoutLargestNonPendingWriter. No test targets the nextPendingFlush window; concurrent stress tests such as the @Nightly testStressConcurrentCommit can reach it only by chance (nextPendingFlush runs after every update, DocumentsWriter.java:406-407, :493-494).
• History. Bugs of the same family, from other root causes, were fixed as bugs: LUCENE-3348 (#4421, 2011), LUCENE-3368 (#4441), LUCENE-8317 (#9364, 2018), LUCENE-8639 (#9685), LUCENE-8813 (#9857, 2019; seen as the sibling assertTicketQueueModification assertion) and #13127/#13571 (seen as maxSeqNo assertions), fixed by #13627 (2024).

The strongest case: DocumentsWriterFlushControl is internal, and the unlocked scan deliberately keeps the flush-control lock off the hot indexing path (contention work such as #12198 and #12958 shows the concern); commit() does not block indexing, so an update racing the cut may land on either side, and the sequence-number finding could be called a linearization question.

Why it fails: the duplicate does not depend on sequence numbers at all, since a reader sees an update's add without its delete, which updateDocument promises cannot happen; the comment, the sibling path, the javadoc and the assertion all say a new-queue DWPT must not flush during a full flush; and the fix keeps the unlocked scan, adding one re-check where the lock is taken anyway (checkOutForFlush is synchronized). An optimisation can justify the unlocked read, not a lost delete. Verdict: not intended, a defect. Cross-model reviewers (OpenAI GPT-5.6) rated the works-as-designed case weak.

Option A was built and run; B and C are untested alternatives.

In the pool scan of DocumentsWriterFlushControl.nextPendingFlush, wrap the check-out in synchronized (this) and call checkOutForFlush only while fullFlush is still false. A skipped DWPT stays flush-pending in the pool; once finishFullFlush clears fullFlush, the next nextPendingFlush (or the next commit's markForFullFlush) flushes it.

if (perThreadPool.isRegistered(next)) {
  synchronized (this) {
    // fullFlush was read before the scan: a full flush may have started since, and a
    // pending DWPT of its new delete queue must wait until that full flush is done
    if (this.fullFlush == false) {
      return checkOutForFlush(next);
    }
  }
}

Trade-offs

• Lock order DWPT -> flush control is the existing one (checkOutForFlush already runs under the DWPT lock and is synchronized); the unlocked scan stays, so indexing threads take the lock no more often than before.
• A pending new-queue DWPT now waits for the commit to finish before it flushes, which is what the sibling checkout() path and the 4.0 CHANGES entry already describe.
• The hunt's first version of the comment said fullFlush "was read without the lock held"; it is read under the lock, which is then released. The diff here rewords the comment; the executable change is the same.

What the checks validated

• This doc's runs: scratch worktree of 645d4fe14a20 with the diff, gradle with the plugin's local test JVM flags, pinned to 2 CPUs, seed DEADBEEF. The minimal test passes all 5,000 commits with assertions off (882 s) and on (1,267 s); on the same worktree before the fix it failed at commit 646 (assertions off, 122 s) and on the doFlush:515 assertion (assertions on, 355 s).
• Unpinned, with the fix: :lucene:core:test for TestIndexWriter*, TestDocumentsWriter*, TestFlushByRamOrCountsPolicy, TestIndexingSequenceNumbers, TestStressNRT, TestStressIndexing, TestStressIndexing2 and TestNRTThreads, assertions on: 30 suites, 371 tests, 28 skipped, 0 failures (without tests.nightly, TestNRTThreads, TestStressIndexing and the @Nightly testStressConcurrentCommit were among the skipped).
• :lucene:core:checkGoogleJavaFormat, ecjLintMain and forbiddenApisMain pass with the fix.
• Independent fact check: the same gradle command with the fix passes (1,076 s, assertions off); standalone with the fix, -da (361 s) and -ea (450 s) pass all 5,000 commits.
• Earlier (hunt, standalone runner with the same hunk): the minimal test passes all 5,000 commits with -da (329 s) and -ea (405 s); the hunt's longer test 10 of 10 x 30 s (-da) and 5 of 5 x 120 s (-ea) in matched runs against 10 of 10 and 5 of 5 failures on stock; 38 tests in TestIndexWriterThreadsToSegments, TestFlushByRamOrCountsPolicy, TestIndexWriterDelete, TestStressNRT, TestDocumentsWriterPerThreadPool and TestDocumentsWriterDeleteQueue ran with 0 failures (the invocation also listed the nightly-only TestNRTThreads, which ran nothing, so the harness labelled it no_tests); nightly, TestNRTThreads and TestIndexWriterDelete (26 tests) pass.
• TLA+ model with the fix: no violation in 49.5M distinct states to depth 38 (not exhaustive).
• Not run: the full Lucene suite, benchmarks. The race runs and the bounded model check support the fix; they do not prove it complete.

On the re-check, when fullFlush is true, call checkoutAndBlock(next) so the DWPT goes to blockedFlushes, as the sibling checkout() does (DocumentsWriterFlushControl.java:256-259).

Trade-offs

Mirrors the sibling path, but must handle a pending DWPT of the old queue found while markForFullFlush is still collecting: it would leave the pool before markForFullFlush sees it and reaches the flush queue only if pruneBlockedQueue(flushingQueue) (:612) runs after the block. Not built.

Run the pool scan of nextPendingFlush inside its synchronized block.

Trade-offs

Simplest to reason about, but puts the flush-control lock on the indexing path whenever buffers are pending, the contention the unlocked scan avoids (#12198, #12958). Not built or benchmarked.

Method: TLA+ model checking (TLC) of the flush protocol, then an executed public-API test (the hunt classes it as semi-formal: the model gave the schedule, the stock-jar test is the proof).

• Model (2026-10-27). A code-reading hypothesis, formed while building a TLA+ model of the full flush (FnbFlush.tla), kept nextPendingFlush's fullFlush read and its pool scan as two steps, as in the code. TLC violated NoBad at depth 27 and TicketOrder at depth 30 (the new-queue ticket queued ahead of the old-queue tickets); the same model with the step atomic is exhaustively clean (13,954,054 distinct states). That unit could not reproduce it on the stock jar and judged it proof-only.
• Execution (2026-10-29). A second unit wrote a public-API JUnit (16 updateDocument threads, a commit() loop, oracles: one live document per key and the commit's sequence-number cut) and ran it on the stock main jars pinned to 2 CPUs: it failed with assertions off and on, and passed with the one-hunk fix in matched runs. The minimal repro above is that test cut down to the duplicate-key oracle.