test(journaling): synchronize legacy recovery repair - #11019
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are isolated to test infrastructure and improve determinism without altering production code paths.
Review tier: Lite
Findings: None
What changed in this PR
This PR makes a flaky journaling recovery test deterministic by removing reliance on timing between the test thread and the journaling manager’s background retry loop. It strengthens the test’s sequencing so it validates both (1) failure leaves state unapplied/unwritten and (2) a subsequent retry can succeed after “repairing” storage, without depending on scheduler behavior.
Changes:
- Updates
RecoverAsync_UnsupportedLegacyRecordCanRetryAfterRepairWithoutPartialApplicationto explicitly coordinate the retry read and recovery phases via a read barrier. - Extends
MutableReadStoragewith an optional “block on Nth read” capability plus synchronization primitives to deterministically gate reads.
| File | Description |
|---|---|
| test/Orleans.Journaling.Tests/StateManagerTests.cs | Makes the legacy recovery retry test deterministic by queuing snapshots and blocking the retry read until the test releases it. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code coverage
Report-only conclusion: regressed. The current-main baseline is commit Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
Problem
RecoverAsync_UnsupportedLegacyRecordCanRetryAfterRepairWithoutPartialApplicationmutated its fake storage after observing the first recovery failure. The journaling work loop can begin retry recovery before that test-thread mutation, so macOS net8 intermittently reread the unsupported record. This is the same scheduler race addressed for earlier recovery retry tests in #10170 and was reintroduced when #10959 added the new legacy-record case.Solution
Queue the unsupported and repaired storage snapshots, block the second read before it observes the repaired snapshot, and explicitly start initialization before asserting the failed state. Releasing the read barrier then completes recovery from the repaired record.
Rationale
The test now directly proves that failed replay leaves state unapplied and storage unwritten, then proves repaired storage recovers successfully. Its outcome depends on explicit recovery phases instead of platform scheduling.
Fixes #10982
Microsoft Reviewers: Open in CodeFlow