Skip to content

fix(cold): fix for spurious test failure#69

Merged
Fraser999 merged 1 commit into
mainfrom
fraser/eng-2297/fix-test
May 25, 2026
Merged

fix(cold): fix for spurious test failure#69
Fraser999 merged 1 commit into
mainfrom
fraser/eng-2297/fix-test

Conversation

@Fraser999
Copy link
Copy Markdown
Contributor

@Fraser999 Fraser999 commented May 25, 2026

Summary

Fixes a spurious failure in fairness_write_serves_before_later_readers (~3% flake rate locally).

The test compared Notify signals fired from caller-side wrappers after truncate_above().await / get_latest_block().await returned. Both signals fire after the writer drops the drain barrier - which is the same instant later readers' permits become available - so the wrapper-task signaling order is racy under multi-thread scheduling even though the underlying semaphore is FIFO-fair.

Fix

Observe ordering inside the backend instead of via post-completion signals. GatedBackend now records each instrumented method into an events: Vec<BackendOp> log at a well-defined point in the call:

  • GetLatestBlock is recorded after the read passes the gate, before its read_sem permit drops.
  • TruncateAbove is recorded after the inner write returns, while the unified handle still holds the drain barrier - so the entry in the log proves the write completed before any later reader could run.

The semaphore forces a strict order on those entries: 64 saturating reads, then the write, then 64 later reads. The test asserts that shape directly, with no Notify and no select! racing the post-drain wake-up window.

Note: BackendOp is an enum so labels are type-safe and future tests get compile-time matching. Only the two methods this test needs are instrumented today; extend BackendOp and the relevant impl when adding tests that need more.

Test plan

  • cargo test -p signet-cold --test concurrency - all 7 tests pass
  • Stress: 100/100 runs of fairness_write_serves_before_later_readers (vs ~29/30 before)
  • cargo +nightly fmt -- --check
  • cargo clippy -p signet-cold --all-features --all-targets
  • cargo clippy -p signet-cold --no-default-features --all-targets

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Fraser999 Fraser999 changed the title fix spurious test failure fix(cold): fix for spurious test failure May 25, 2026
@Fraser999 Fraser999 requested review from Evalir and prestwich May 25, 2026 11:21
Copy link
Copy Markdown
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. appreciate the thoroughness on the doc

/// appears in the log, the write has completed and no later reader can
/// yet be running. The recording is therefore strictly ordered by the
/// semaphore, without racing the post-drain wake-up window where reader
/// and writer wrappers would otherwise compete to signal downstream.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense to me

@Fraser999 Fraser999 merged commit e4facc1 into main May 25, 2026
8 checks passed
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.

3 participants