Skip to content

fix(tests): deflake flush-observation tests + tolerate concurrent sweep race - #330

Merged
JustMaier merged 1 commit into
mainfrom
fix/deflake-flush-observation-tests
Jul 17, 2026
Merged

fix(tests): deflake flush-observation tests + tolerate concurrent sweep race#330
JustMaier merged 1 commit into
mainfrom
fix/deflake-flush-observation-tests

Conversation

@JustMaier

Copy link
Copy Markdown
Contributor

Problem

A family of #[serial] concurrent_engine tests observe post-flush state after a fixed thread::sleep or after wait_for_flush(alive_count). Both under-synchronize: the flush loop publishes the snapshot and bumps alive_count/publish_count before it does the post-publish disk I/O — there's a deliberate std::thread::yield_now() between the publish and the ops-log append (concurrent_engine.rs:2937). Under scheduler contention (default --test-threads, CI, or a concurrent bulk load) the observed state hasn't landed yet, so the assert races.

The victim rotates by scheduling: CI hit test_flush_thread_appends_ops_to_shard_stores (at two different asserts across runs); locally under load I reproduced test_sync_filter_values_clear_all, test_cursor_persists_via_merge_thread, test_put_bulk_persists_to_docstore, test_put_bulk_loading_then_persist, test_delete_cleans_filter_and_sort_bits, and test_get_documents_preserves_order. All the same root cause.

src/concurrent_engine.rs is byte-identical to main — this flake is not caused by any recent feature diff (it originally surfaced blocking PR #328, whose diff never touches this file).

Fixes

Test synchronization (concurrent_engine.rs):

  • Add wait_until(max_ms, cond) — poll a condition instead of sleeping.
  • Disk/docstore-observation tests now poll the actual asserted condition (ops_count > 0, filter/sort shard present, cursor persisted, all docs readable) with a generous deadline.
  • In-memory-snapshot tests use wait_for_flush_quiet / wait_for_flush(0) instead of a bare sleep or a wait_for_flush(count) that never changes on an upsert.

Production robustness (shard_store.rs) — a real bug found while deflaking:
Opening a ShardStore runs a recursive orphan-.new sweep. When it races a live writer's compaction rename (packed sort shards are written as <field>.new then renamed on every sort flush), the vanished file returns NotFound, which sweep_dir counted as a hard failure → ShardStore::new returned Err → the test's .unwrap() panicked with sweep failed for 1 orphan .new file(s). NotFound means the orphan is already gone (the desired end state), so it's now treated as success in both the promote-rename and delete branches. Idempotent and correct for the crash-recovery startup sweep too.

Verification

Full lib test suite (--skip test_min_tracked_value_after_expansion, known deadlock) run under a concurrent 107M dump (heavy IO/CPU amplifier):

branch baseline this PR
full-suite runs failing 3 / 4 0 / 4

The sweep fix is what closes the last gap — after the test-sync fix alone, test_flush_thread still failed 3/4 at the sweep race; with both, 4/4 pass.

🤖 Generated with Claude Code

…ep race

A family of #[serial] concurrent_engine tests observe post-flush state
(on-disk ops-log appends, merge checkpoints, docstore write-through, or
cleared bits) after a fixed thread::sleep or after wait_for_flush(alive_count).
Both under-synchronize: the flush loop publishes the snapshot and bumps
alive_count/publish_count BEFORE it does the post-publish disk I/O (there is
a deliberate yield_now between). Under scheduler contention (default
--test-threads, CI, or a concurrent bulk load) the observed state has not
landed yet, so the assert races and fails on a rotating cast of tests
(CI hit test_flush_thread_appends_ops_to_shard_stores at two different
asserts; locally test_sync_filter_values_clear_all, test_cursor_persists,
the put_bulk pair, test_delete_cleans, test_get_documents_preserves_order).

Fixes:
- Add wait_until(max_ms, cond): poll a condition instead of sleeping.
- Disk/docstore-observation tests poll the actual asserted condition
  (ops_count>0, filter/sort shard present, cursor persisted, docs readable).
- In-memory-snapshot tests use wait_for_flush_quiet / wait_for_flush(0)
  instead of a bare sleep or a wait_for_flush(count) that never changes on
  an upsert.

While deflaking, the flush-thread test surfaced a real concurrency bug:
opening a ShardStore instance runs a recursive orphan-.new sweep, and when
it races a live writer's compaction rename the vanished file returns
NotFound, which sweep_dir counted as a hard failure -> ShardStore::new
returned Err. NotFound means the orphan is already gone (the desired end
state), so treat it as success in both the promote-rename and delete
branches. Idempotent and correct for the crash-recovery startup sweep too.

Verified: full lib suite under a concurrent 107M dump went from 3/4 runs
failing (this family) to 4/4 passing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JustMaier
JustMaier merged commit 5dc7c2e into main Jul 17, 2026
4 of 5 checks passed
JustMaier added a commit that referenced this pull request Jul 17, 2026
…ortAt, model3dId (#332)

* docs: scheduled-publish wrap-up — per-image fan-out, ingested sortAt, model3dId (#13)

Documents the shipped scheduled-publish effort (PRs #325/#326/#328/#329/#330)
across the doc set:

- pg-sync-v2-final.md: Post fan-out is now per-image materialized
  (type: fan_out_per_row) not queryOpSet; shared PG functions
  bitdex_post_fanout_ops / bitdex_image_sortat_ops; sortAt ingested (computed:
  dropped), dump never trusts the column; ModelVersion/Model stay queryOpSet.
- CLAUDE.md: Sync V2 section reflects new fan-out shape + ingested sortAt +
  model3dId.
- scheduled-publish-design.md: §1 history corrected (2024 migration triggers,
  Post-side neutered by name-collision, new_image_sort_at absent from prod
  pg_trigger per zuri's live verification); §2.D marked EXECUTED with the
  no-backfill decision and W1-4 dominant-class (visible-but-misordered) finding.
- execution-plan: W4 step 2 no-backfill correction note.
- FOLLOWUP.md: 7 residuals (over-cap fan-out skip, flaky flush tests, retired
  combinedNsfwLevel, frankenstein docs, MV cascade cost, stage_dir isolation,
  E2E population gap).
- HANDOFF.md: scheduled-publish design chain pointer + W4 rollout state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(reemitter): correct Tf-activation model to sweep-driven shadow flip

Prod evidence (sortat-divergence-specimens-2026-07-16.md PR-m5 + execution
plan corrected [PR-B2]) established that scheduled slots are ALIVE with
isPublished=false (quarantine branch), NOT deferred-map-resident, so Tf
activation is an overdue-sweep shadow flip, not activate_due. The re-emitter is
therefore a co-driver of activation, not a pure safety net. W3 confirmed the
no-op case (sweep alone flipped isPublished + preserved ingested sortAt, 14/14
E2E). Fixes fact #1 + the §7 'does not drive activation' bullet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
JustMaier added a commit that referenced this pull request Jul 17, 2026
… deadlines (#331)

Follow-up to #330. Its own CI run still failed test_flush_thread_appends_
ops_to_shard_stores at the filter-bucket assert: the wait_until(...) guarding
that disk read discarded its return value, so a deadline miss on a slow
shared runner fell through to `filter_store.read()` returning None and
panicked with the generic "filter bucket should exist after insert" — which
can't be told apart from a real write failure.

- Every disk-observation wait_until now asserts its bool with a timeout-
  specific message, so a deadline miss says "timed out waiting for ..."
  instead of masquerading as a missing shard. (Two sites — filter and sort —
  previously discarded the return entirely.)
- Raise disk-observation deadlines 15s -> 60s. CI runners are far slower than
  a 32-core dev box (the suite takes 5+ min there); these are correctness
  waits, not latency contracts, and wait_until returns the instant the
  condition holds, so a generous ceiling is free on healthy runs.

Verified: flush_thread 15/15 isolated; full lib suite 2/2 clean.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant