fix(cold): fix for spurious test failure#69
Merged
Conversation
Evalir
approved these changes
May 25, 2026
Member
Evalir
left a comment
There was a problem hiding this comment.
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. |
prestwich
approved these changes
May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Fixes a spurious failure in
fairness_write_serves_before_later_readers(~3% flake rate locally).The test compared
Notifysignals fired from caller-side wrappers aftertruncate_above().await/get_latest_block().awaitreturned. 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.
GatedBackendnow records each instrumented method into anevents: Vec<BackendOp>log at a well-defined point in the call:GetLatestBlockis recorded after the read passes the gate, before itsread_sempermit drops.TruncateAboveis 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
Notifyand noselect!racing the post-drain wake-up window.Note:
BackendOpis an enum so labels are type-safe and future tests get compile-time matching. Only the two methods this test needs are instrumented today; extendBackendOpand the relevant impl when adding tests that need more.Test plan
cargo test -p signet-cold --test concurrency- all 7 tests passfairness_write_serves_before_later_readers(vs ~29/30 before)cargo +nightly fmt -- --checkcargo clippy -p signet-cold --all-features --all-targetscargo clippy -p signet-cold --no-default-features --all-targets