tryDeleteDocument and tryUpdateDocValue during commit() are numbered after the commit, yet land in it

Every IndexWriter change returns a sequence number, and commit() promises that all numbers at or below its own are in the commit and all others are not. tryDeleteDocument and tryUpdateDocValue, the expert calls that change one document by its docID in an NRT reader, break that promise when they run while another thread is inside commit(): in the repro they return 8, the commit returns 7, and the change is in commit 7. The term-based deleteDocuments(Term), deleteDocuments(Query) and updateNumericDocValue(Term) in the same window also get 8 and correctly stay out. The cause is that the shared tryModifyDocument takes its number from the new delete queue that the commit's full flush has already swapped in, and applies the change straight to the pooled segment reader, which the commit then writes into its snapshot. Applications that use the commit's number as an exact cut (rollback, replay, replication) get a wrong cut; with rollback() such a change survives. It is over-inclusion only: in every run, nothing numbered at or below a commit was missing, and CheckIndex was clean where it ran. Every release since 6.2.0 (7.4.0 for tryUpdateDocValue) and main have it.

• Sequence number (seqNo): every IndexWriter method that changes the index returns one. The class javadoc says it "expresses the effective order in which each change was applied", and commit() returns one "describing which changes are in the commit point and which are not" (IndexWriter.java:117-119).
• try* methods: tryDeleteDocument(reader, docID) and tryUpdateDocValue(reader, docID, fields) change one document by its docID in an NRT reader of this writer. They return a seqNo, or -1 when they cannot apply the change. tryDeleteDocument also returns a number when the document was already deleted. The tryUpdateDocValue javadoc says a null value can be used to un-delete a soft-deleted document.
• Full flush: what commit() (and an NRT reopen, and flush()) does first: it fixes the commit's seqNo, starts a new delete queue whose numbers are higher, and writes the documents buffered at that moment to segments.

Without a concurrent commit the try* calls behave: a try delete before the commit gets a number at or below it and is in it; one after gets a higher number and is not (the JUnit's control tests tryDeleteDocument; the rollback probe's and the release probes' before/after controls show the same for tryUpdateDocValue).

1. The commit fixes its number and swaps the queue first. prepareCommitInternal takes fullFlushLock (IndexWriter.java:3806) and calls flushAllThreads() (:3809). Under the DocumentsWriter monitor (DocumentsWriter.java:674-682) this runs markForFullFlush (DocumentsWriterFlushControl.java:557-575): fullFlush = true, and resetDeleteQueue (DocumentsWriter.java:588-598) makes the old queue's maximum the commit's number and installs a new queue that starts above it (DocumentsWriterDeleteQueue.java:616-630).
2. Then it flushes without holding the writer's monitor. It writes the buffered segments and, at the end of the flush, publishes them (DocumentsWriter.java:580 → IndexWriter.java:449; :3819 publishes any leftover), then applies deletes (:3825). It holds fullFlushLock throughout, and the writer's monitor only briefly to publish each segment (:2953), until it takes it for the snapshot at :3826.
3. A try* call runs in that gap. Both methods are synchronized on the writer (:1688, :1721) and share tryModifyDocument (:1791-1827). It applies the change directly to the pooled segment reader (:1823) and returns docWriter.getNextSequenceNumber() (:1824), which now comes from the new queue: 8, above the commit's 7. It never takes fullFlushLock and never checks whether a full flush is running, so any try* call that returns after the queue swap and before :3826 is caught.
4. The commit then snapshots the change. Back under the writer's monitor (:3826) it calls writeReaderPool(true) (:3827), which writes the pooled readers' pending deletes and doc-values updates, the try* change included, and clones the segment infos into the commit point (:3850). The comment there places the clone so that partial compound changes (a delete without the add of an updateDocument) cannot "sneak into the commit point" (:3845-3849).
5. Why the term-based calls are fine. They go through DocumentsWriter.applyDeleteOrUpdate (DocumentsWriter.java:153-165), synchronized "to make sure we don't replace the deleteQueue while applying this update / delete": number and change land in the same new queue, which this commit does not freeze.

The window runs from the queue swap to the commit's final take of the writer's monitor (:3826) and includes writing the buffered segments, so it is wide: a stress test without any hook hits it constantly.

• Who is hit. Applications that call tryDeleteDocument or tryUpdateDocValue while another thread runs commit() or prepareCommit(), and that act on the commit's number. LUCENE-7302 added sequence numbers so that callers "can know exactly which operations made it into the commit and which did not". Typical cuts: aborting with rollback(), documented to return to "the state ... when commit() was last called" (IndexWriter.java:2567-2571); replaying a transaction log from the commit's number; reconciling a replica or a system of record at that cut. Sequence numbers are transient and valid only within one IndexWriter instance (IndexWriter.java:119-121), so the cut matters while that writer lives. prepareCommit() shares the code path (read from the source; no executed test calls it directly).
• What goes wrong (executed on main with assertions on and off, 10.5.2 and 9.12.3). After rollback(), a tryDeleteDocument numbered 9 above a commit numbered 8 is still applied, so a document the application believes live is gone; a tryUpdateDocValue un-delete of a soft-deleted document (9 against 8) survives, so the document is visible to searches. Before/after controls behave; CheckIndex is clean.
• What does not go wrong. Idempotent replay from the commit's number still converges (it re-applies a change that is already there). In every run, no operation numbered at or below a commit was missing from it, and CheckIndex found no corruption where it ran. rollback() itself does what its javadoc says; what is wrong is the number that labels the change as outside the commit.
• Default or opt-in. No explicit codec or directory override: the portable probe ran with the normal default codec and a plain NIOFSDirectory, and the JUnit with the test framework's randomized codec and directories (for example Asserting(Lucene104)). The repro only sets NoMergePolicy, which keeps the try* javadoc's success condition (the segment "has not been merged away") true; merges (and with them the merged-away -1 path) were not exercised, while fully deleted segments that the try delete drops were (the finder's one-document repro, the independent reproduction's stress test). The opt-in is the expert try* API. Read-only code reads (default branches, not exhaustive): Elasticsearch's engine writer fails tryDeleteDocument only when assertions are enabled (otherwise it calls through to Lucene), OpenSearch's engine writer always rejects it, and an OpenSearch sandbox plugin calls it but checks only for -1 and never uses the sequence number; no tryUpdateDocValue caller was found in Elasticsearch, OpenSearch or Solr; custom applications were not surveyed.
• How often. Two documents suffice, and the repro failed on every run (40 of 40 try* runs over 20 seeds). Without hooks, a stress test that commits in a loop while another thread try-deletes mislabelled 3,417 of 5,981 successful try deletes in one run and 14,698 of 20,992 over five runs; every mislabelled call overlapped its commit in wall-clock time.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: tryDeleteDocument seqNo=7, commit seqNo=6, in commit expected:<false> but was:<true>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:120)
       org.apache.lucene.index.TestTryDeleteDocumentDuringCommit.testSeqNoAboveCommitButDeleteInCommit(TestTryDeleteDocumentDuringCommit.java:79)

• The contracts. commit() (IndexWriter.java:4203-4205): "it returns the sequence number such that all indexing operations prior to this sequence will be included in the commit point, and all other operations will not"; @return (:4208-4210, repeated for prepareCommit() at :3730-3732): "All sequence numbers <= this value will be reflected in the commit, and all others will not." The class javadoc (:117-119) covers "each method that changes the index". The try* javadoc (:1680-1681, :1710-1711) promises "a valid (> 0) sequence number". IndexWriter is "completely thread safe, meaning multiple threads can call any of its methods, concurrently" (:167-168). CHANGES.txt:8925-8927 (LUCENE-7302): methods that change the index "now return a long "sequence number" indicating the effective equivalent single-threaded execution order".
• Upstream tests. TestIndexingSequenceNumbers checks exactly this boundary under concurrent commits, for the queue-based operations only; it never calls a try* method. Seven test classes call try* (TestTryDelete, TestRollingUpdates, TestIndexWriter, TestIndexWriterDelete, TestInfoStream, TestSoftDeletesRetentionMergePolicy, TestMixedDocValuesUpdates); none compares a try* number with a commit's, and the two that run try* alongside commits use the number only to retry on -1.
• History and precedent. LUCENE-7302 (2016) turned tryDeleteDocument from boolean into this seqNo; the violated javadoc came three days later in the same series, before any release. Maintainers fixed full-flush seqNo inconsistencies as bugs before: LUCENE-9304 (2020, which changed this very return line while fixing getMaxCompletedSequenceNumber) and the seqNo races of #13127 and #13571, fixed by #13169 (first in 9.11.0) and #13627 (first in 9.12.0). On java-user@ in 2016 the LUCENE-7302 author wrote: "all sequence numbers <= the sequence number returned from IW.commit "made it" into the index, and all other ops did not make it."
• The code's own guards. DocumentsWriter.applyDeleteOrUpdate (DocumentsWriter.java:153-165) is synchronized "to make sure we don't replace the deleteQueue while applying this update / delete"; deleteAll takes fullFlushLock for "point in time semantics" (IndexWriter.java:2757, :2764); prepareCommitInternal clones while it holds fullFlushLock and the monitor so that partial compound changes cannot "sneak into the commit point" (:3845-3849; a comment about operations such as updateDocument, not a general rule).

The strongest case for "intended", built by the devil's-advocate check: try* is a best-effort expert API; LUCENE-4521 (2012) deliberately made a successful try delete reach the next commit; LUCENE-1335 (2008) declined API promises about concurrent commit, close and addIndexes; the try* javadoc does not link the #sequence_number anchor; and the open #7444 (2015) calls tryDeleteDocument "a trap" and proposes removing it.

Why it fails: inclusion in the commit is fine, but returning a number that says "not in this commit" for a change that is in it is not; the try* numbers come from the same counter as every other operation, and nothing says they are weaker. LUCENE-1335 predates sequence numbers and try*, names admin methods only, and is not in today's docs; the 2016 javadoc is exactly such a promise about concurrent commits. LUCENE-7302 gave try* its number, and its CHANGES entry covers every method that changes the index. #7444 is about rare false returns before sequence numbers existed: it may shape the fix (deprecation), not whether this is a bug. Verdicts: devil's advocate "real, high"; the cross-model works-as-designed review rated the intended argument "weak"; the adjudication "confirmed, high", for the commit/prepareCommit boundary only.

How strong the case still is: a read-only fact check by OpenAI GPT-5.6 rated it moderate overall and weak under the literal javadocs. It is strongest for tryDeleteDocument (an expert, best-effort API; LUCENE-4521 made inclusion in the next commit intentional; the index state matches the serial order "try, then commit", only the number does not) and weaker for tryUpdateDocValue, whose javadoc advertises the soft-delete un-delete. Maintainers may raise these points; the answer is the explicit @return contract and the same-window asymmetry.

Two code fixes were built and validated in a scratch worktree of ea57360912ed (removed afterwards); two more options are documented. Taking fullFlushLock inside the synchronized try* methods would invert the existing lock order, so both code fixes avoid that.

Drop synchronized from tryDeleteDocument and tryUpdateDocValue and route both through a helper that takes fullFlushLock and then calls the still-synchronized tryModifyDocument. A try call that arrives during a commit, NRT reopen or flush() waits for it to end, then gets a number above the commit's and is not in it. Patch: +22/-5 in IndexWriter.java.

public long tryDeleteDocument(IndexReader readerIn, int docID) throws IOException { // was synchronized
  return tryModifyDocumentOutsideFullFlush(readerIn, docID, (leafDocId, rld) -> { /* unchanged */ });
}
// tryUpdateDocValue: the same; buildDocValuesUpdate stays under synchronized (this)

private long tryModifyDocumentOutsideFullFlush(
    IndexReader readerIn, int docID, DocModifier toApply) throws IOException {
  assert Thread.holdsLock(this) == false : "fullFlushLock must be taken before the IW monitor";
  synchronized (fullFlushLock) { // same order as commit, getReader and deleteAll
    return tryModifyDocument(readerIn, docID, toApply); // still synchronized on the writer
  }
}

Trade-offs

• A try call now blocks for the rest of a running full flush (writing the buffered segments). Not benchmarked.
• A caller that calls try* while holding the writer's monitor (the class javadoc already says not to synchronize on the writer) would now risk a deadlock; the added assertion catches it under -ea (with assertions off it would deadlock against a full flush), and no in-tree test trips it. One callback passed: TestSoftDeletesRetentionMergePolicy calls tryDeleteDocument from a merged-segment warmer and is among the 18 closest suites; broader callback and full-flush interactions were not evaluated.
• The public methods lose the synchronized modifier; they still take the monitor inside.

What the checks validated

• The repro passes 5 of 5 with assertions on and off and 50 of 50 with tests.iters=10, through gradle and the harness: both try calls return 8 and stay out of commit 7.
• All of :lucene:core:test: 748 classes, 8,870 tests, 322 skipped, 0 failures. All of :lucene:misc:test (includes TestIndexRearranger): 315 tests, 9 skipped, 0 failures. The 18 closest core suites also with assertions off: 359 tests, 0 failures. :lucene:core:check -x test passes.
• A hook-free stress test (commits back to back while another thread try-deletes): 0 mislabelled in five runs, with 1 to 6,667 successful try deletes per run, the same range as stock.

Take the seqNo first, atomically with the start of a full flush (a new DocumentsWriter method, synchronized like the queue swap, returns -1 while isFullFlush()), and apply the change only if a number was given. -1 is already the documented outcome that sends callers to delete by Term or Query; the javadoc gains "or while a commit, NRT reopen or flush is flushing all buffered documents". Patch: +13 in DocumentsWriter.java, +18/-8 in IndexWriter.java.

// DocumentsWriter
synchronized long getNextSequenceNumberOutsideFullFlush() {
  if (flushControl.isFullFlush()) {
    return -1;
  }
  return deleteQueue.getNextSequenceNumber();
}
// IndexWriter#tryModifyDocument, inside synchronized (bufferedUpdatesStream)
final long seqNo = docWriter.getNextSequenceNumberOutsideFullFlush();
if (seqNo == -1) {
  return -1;
}
toApply.run(docID, rld);
return seqNo;

Trade-offs

• Measured: in the hook-free stress test with back-to-back commits, two of five runs had 0 successful try deletes and the others 226 to 3,945. Under frequent commits or NRT reopens many calls fall back to the term path, and a caller that retries on -1, as the tryUpdateDocValue javadoc suggests, could spin for the length of a flush.
• It also refuses during NRT reopens, which make no seqNo exclusion promise. A narrower variant (not tested) would refuse only during a commit's full flush.
• IndexRearranger treats -1 as "should never happen"; it does not commit concurrently, so it is not hit.

What the checks validated

• The repro passes (both try calls return -1 and change nothing) with assertions on and off and 50 of 50 iterations.
• The 18 closest core suites (359 tests) and TestIndexRearranger pass. The full suites were not run with A.

Say in the try* javadoc (or in the commit() @return) that a try* change made while a commit runs may be in that commit even though its number is higher.

Trade-offs

Cheapest, no behaviour change; leaves the rollback and cut problems to applications. Not tested.

The direction of the open #7444 ("remove or fix IndexWriter.tryDeleteDocument").

Trade-offs

Larger API change; maintainers may prefer to answer there. Not tested.

Method: TLA+ model checking (TLC), a hunt unit (2026-10-26), on main ea57360912ed.

• The unit's plan asked for a TLA+ model of DocumentsWriterFlushControl, the per-thread writer pool, the delete queue (slices, seqNo assignment, queue swap on full flush) and the flush queue, with indexing threads, an NRT refresher and a committer, and for the invariant "an NRT reader or commit with seqNo s contains exactly the ops with seqNo <= s (the commit javadoc)". It did not name the try* methods; its dedup list named two other try* rows (LFV-009, LFV-010).
• The finder's model (DWFlush.tla, about 500 lines with a file:line map) added tryDeleteDocument as an action that takes its number from the current queue under the writer's monitor only. With configuration MC_D_try (two indexers, one try delete on a published document, NRT reopen and commit), TLC found the invariant violated in a 20-state trace (112,706 states generated, 38,380 distinct).
• The finder replayed the trace as a public-API JUnit on the stock jars (two schedules: an InfoStream message and a FilterDirectory#createOutput hook), which failed with 8 against 7. Independent checks followed (see Checks), and this doc added the minimal five-test version and the fix validation.