Sync: create-then-void/delete race leaves stale voided documents searchable (delete is not version-ordered like upsert)

[dkayiwa]

Summary

A record that is created and then voided/deleted in quick succession can be left in the read store as a stale, still-searchable document, because the async sync pipeline's delete is not version-ordered the way upsert is. The delete can execute before the create's (slower) index write lands, so the delete no-ops and the subsequent upsert resurrects a document for a record that is actually voided/gone.

This is a sync-layer eventual-consistency defect. It is not in any serializer or in a specific ResourceTypeProvider — the disposition logic (voided → delete) is computed correctly. It affects every resource type; it was simply easy to trigger via bill/billDiscount/billRefund REST calls.

Environment

• querystore 1.0.0-SNAPSHOT, platform 2.9.0-SNAPSHOT, Lucene backend
• Reference Application standalone 3.7.0-rc.2
• Observed while verifying the billing → querystore integration (billing-querystore omod), but the racing machinery is entirely in querystore's sync package.

Root cause

The write and delete halves of the pipeline are asymmetric with respect to the conditional-upsert-by-version invariant (ADR Decision 3):

• BackendStore.upsert(doc) is version-guarded — a write whose last_modified is older than the stored version is dropped (BackendStore.java:51-56; Lucene/MySQL/ES all honor it, e.g. ES external_gte). This makes upsert-vs-upsert monotonic per resource_uuid: freshest wins regardless of execution order.
• BackendStore.delete(resourceType, resourceUuid) is unconditional (BackendStore.java:58). The Lucene impl is a bare term delete with no version predicate:

  // LuceneBackendStore.java:158-161
  public WriteResult delete(String resourceType, String resourceUuid) {
      ...
      writer.deleteDocuments(new Term(LuceneFieldNames.RESOURCE_UUID, resourceUuid));
  }

  There is no versioned tombstone. A delete carries no logical timestamp, so it cannot "win" against a concurrent-or-later upsert.

Now combine that with how tasks are scheduled:

1. RecordProjector.project(...) correctly routes a voided node to toDelete and a live node to toIndex, then hands the work to AfterCommitDispatcher.dispatch(...).
2. AfterCommitDispatcher.dispatch registers an afterCommit() synchronization that does executor.submit(guarded) — one submission per source transaction.
3. SyncExecutor is a multi-worker Executors.newFixedThreadPool(poolSize, ...) (SyncExecutor.java:66) with no per-resource_uuid ordering.
4. RecordIndexer.index(doc) computes an embedding first (embeddingProvider.embed(...)) before writing — a comparatively slow step — while RecordIndexer.delete(...) is a fast term delete.

So for create record R (txn A commits → submit upsert(R, v1)) followed shortly by void/delete record R (txn B commits → submit delete(R)):

• the two tasks run on different pool threads, unordered;
• delete(R) finishes first (fast, and/or scheduled first) and removes nothing — the doc isn't written yet;
• upsert(R, v1) then completes the slow embed and writes the doc;
• the version guard does not save us: there is no newer versioned record to compare against (the delete left no tombstone), so v1 is written and survives.

Net: a voided/deleted record remains indexed and searchable.

Why the ADR's self-healing argument doesn't cover this

AfterCommitDispatcher.wrap(...) documents (per ADR Decision 12) that a missed projection is "corrected by the next save or the bootstrap pass — neither overwrites the freshest document." That holds for a lost upsert (under-indexing, repaired by the next save). It does not hold for a lost delete: a voided/purged record is terminal — there is no "next save" — so the stale document persists until the reconciliation path (GET /querystore/drift + POST /querystore/reindex) is run. It is silent over-indexing, not self-healing.

Reproduction (observed)

On the standalone above, with the billing types (any type works — see below):

Fast path — race fires:

1. POST /billing/bill (create) then, within ~1s, DELETE /billing/bill/{uuid} (void).
2. GET /querystore/drift → billing_bill shows indexedCount = coreCount + 1, drift = -1.
3. The voided bill is still returned by POST /chartsearchai/search for that patient (a clinician sees a cancelled bill).

Concrete run: created bill 0009-1, voided its refund and then the bill back-to-back; final drift reported billing_bill core=3 index=4 — the voided bill stayed. A chartsearchai query then listed it: "Bill 0007-5: 300.00" for a bill that was voided=1 in the DB.

Settled path — no race:

1. POST /billing/bill; poll /querystore/drift until the index reflects the create (index write has landed).
2. DELETE /billing/bill/{uuid}.
3. /querystore/drift returns to drift = 0 — the delete purges correctly every time.

The only variable is whether the create's async index write has landed before the delete is dispatched. This is the signature of a create-then-delete ordering race, not a logic error.

Not billing-specific / not a serializer bug:

• SyncExecutor, AfterCommitDispatcher, RecordProjector, RecordIndexer, BackendStore.delete are all type-agnostic.
• A SQL-forced PAID bill with no child voided and purged cleanly, ruling out entity state as the cause.
• The same rapid create→delete on obs/condition/etc. is expected to reproduce identically; only the ease of firing two quick REST writes differs.

Impact

• Correctness: a voided/deleted clinical record can remain searchable in the read store until a reindex. For a clinician-facing chart search this is a trust/safety issue (surfacing a cancelled bill, a deleted obs, etc.).
• Likelihood: low at human interaction speed (seconds between create and void let the index settle). Higher under automated/bulk writers, scripted corrections, imports, or double-click/void-immediately UX.
• Backend scope: the asymmetry (versioned upsert, unversioned delete) is in the shared contract; Lucene confirmed, MySQL/ES have the same unconditional delete.

Detection & workaround (today)

• Detect: GET /querystore/drift — a negative drift (index > core) flags exactly this class of stale extra.
• Repair: POST /querystore/reindex {"patient":"<uuid>"} (per patient) or {"scope":"all"} — delete + rebuild from non-voided core. Confirmed to clear the stale doc.

Suggested fixes (for discussion)

1. Versioned tombstone / conditional delete. Give delete the same version semantics as upsert: a void/delete writes a tombstone carrying the source last_modified, and upsert refuses to resurrect over a >= tombstone (and delete refuses to remove a strictly-newer doc). This extends the existing conditional-by-version invariant to cover the terminal op and makes per-resource_uuid writes monotonic regardless of execution order. Preferred — it keeps the lock-free, unordered executor.
2. Per-resource_uuid ordering. Route all pipeline ops for a given (resourceType, resource_uuid) through a single ordered lane (hash the uuid to a fixed worker, or a per-key sequential executor) so submission order = execution order. Simpler to reason about; costs some parallelism and needs care with the embed step.
3. Re-check liveness at write time. Before the terminal upsert, re-verify the source record is still non-voided/extant. Weakest (still racy, adds a read), noted only for completeness.

Pointers

• api/.../sync/RecordProjector.java — project(...) disposition + dispatch
• api/.../sync/AfterCommitDispatcher.java — dispatch(...) → executor.submit(...)
• api/.../sync/SyncExecutor.java:66 — newFixedThreadPool(poolSize, ...)
• api/.../sync/RecordIndexer.java — index(...) (embed-then-upsert) vs delete(...)
• api/.../backend/BackendStore.java:51-58 — versioned upsert vs unconditional delete
• api/.../backend/lucene/LuceneBackendStore.java:158-161 — unconditional term delete