Skip to content

fix(journaling): serialize recovery retries and preserve persistence failures - #11326

Merged
ReubenBond merged 11 commits into
dotnet:mainfrom
ReubenBond:rb-journaled-state-lifecycle
Sep 21, 2026
Merged

ReubenBond merged 11 commits into
dotnet:mainfrom
ReubenBond:rb-journaled-state-lifecycle

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Problem

An initial recovery error permanently fences the journal manager even though recovery can start again using the existing state reset/replay contract. Owner shutdown can also be reported as a terminal journal failure, and a secondary error during failure handling can overwrite the original cause.

Changes

  • Allow an explicit subsequent InitializeAsync to retry failed recovery on the same manager and registered states. Each failed attempt reports its error to that attempt's callers; the next attempt resets recovery bookkeeping and replays from the beginning.
  • Use one work-loop task for the manager's lifetime. Failed recovery leaves that task waiting for a newly queued initialization request, so retries never replace or overlap work-loop tasks. The existing uninitialized state is sufficient; no separate RecoveryFailed state is needed. Leftover signals do not trigger automatic retries.
  • Keep writes and deletion unavailable until initialization succeeds and state registration closed after initialization begins. Concurrent initialization callers share the active attempt. Check shutdown and admit initialization under the same lock used by stop.
  • Handle owner-canceled recovery and idle waits through normal shutdown cleanup. Actual write/delete failures remain terminal: preserve the first failure, complete manager waiters with that cause, and request grain deactivation. Caller cancellation ends its wait while the manager retains ownership of recovery or persistence.
  • Clarify initialization, owner-managed deletion quiescence, synchronous safe-to-commit updates, and explicit codec construction in the lifecycle guidance.

Coverage includes the actual work-loop task identity across repeated failures, eight concurrent retry callers, successful recovery, persistence and disposal; explicit retry versus idle shutdown; transient I/O and provider cancellation; repaired partial replay without duplicate list entries or unknown streams; retired-state cleanup; and existing persistence-failure, cancellation and ACK guarantees. The zero-byte write test explicitly distinguishes the initial directory-only flush from a subsequent empty write.

Scope

The public Journaling API remains unchanged. Recovery retries use the existing Reset and replay contracts; the runtime trusts state-machine implementations. Retry timing belongs to the caller. Application and feature code owns preparation, dependencies, and cleanup; the journal manager owns initialization coordination, persistence failures, and shutdown.

Messaging consumer adaptations are tracked separately in #11282, #11284, #11285, and #10693.

Current foundation diff.

Microsoft Reviewers: Open in CodeFlow

Copilot AI lite review requested due to automatic review settings September 18, 2026 18:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Active work can remain incomplete when cancellation occurs during preparation or storage awaits, potentially leaving callers blocked.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Adds lifecycle hooks and manager-owned codec resolution for journaled states, with tests, API updates, and documentation.

Changes:

  • Adds readiness, validation, deletion, and terminal-failure hooks.
  • Resolves codecs using the manager’s configured journal format.
  • Expands lifecycle and cancellation regression coverage.
File Description
test/​Orleans.Journaling.Tests/​StateManagerTests.cs Enables partial test organization.
test/​Orleans.Journaling.Tests/​StateManagerLifecycleTests.cs Adds lifecycle regression coverage.
test/​Orleans.Journaling.Tests/​KeyedJournalingRegistrationTests.cs Tests format-specific codec resolution.
test/​Orleans.Journaling.Tests/​JournaledGrainCompositionTests.cs Updates test state readiness setup.
src/​Orleans.Journaling/​JournaledStateManager.cs Implements lifecycle hooks and codec resolution.
src/​Orleans.Journaling/​IJournaledStateManager.cs Adds codec-resolution API.
src/​Orleans.Journaling/​IJournaledState.cs Defines lifecycle hook contracts.
src/​api/​Orleans.Journaling/​Orleans.Journaling.cs Updates the generated API surface.
docs/​site/​src/​content/​docs/​grains/​journaling/​runtime-behavior.md Documents lifecycle behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Orleans.Journaling/JournaledStateManager.cs
@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request
Lines 82.91% (115,363 / 139,145)
Branches 72.21% (33,479 / 46,361)

Report-only conclusion: current-main baseline stale.

The newest successful coverage run tested 8efd435, not current main a68e9ed.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 19, 2026 00:19
@ReubenBond
ReubenBond force-pushed the rb-journaled-state-lifecycle branch from a7c457d to 93fb647 Compare September 19, 2026 00:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 20, 2026 18:42
@ReubenBond
ReubenBond force-pushed the rb-journaled-state-lifecycle branch from 93fb647 to 0237ba4 Compare September 20, 2026 18:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 20, 2026 22:09
@ReubenBond ReubenBond changed the title feat(journaling): add journaled state lifecycle hooks feat(journaling): add synchronous state lifecycle validation Sep 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Address the missing lifecycle API/implementation and clarify the standalone execution-thread documentation.

Review effort: Lite
Findings: None

@ReubenBond ReubenBond changed the title feat(journaling): add synchronous state lifecycle validation feat(journaling): expose manager codecs and refine failure handling Sep 21, 2026
Copilot AI review requested due to automatic review settings September 21, 2026 16:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The manager codec API described by the pull request is missing; documentation also needs a minor correction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)

Comment thread src/Orleans.Journaling/IJournaledStateManager.cs Outdated
@ReubenBond ReubenBond changed the title feat(journaling): expose manager codecs and refine failure handling fix(journaling): preserve failure and shutdown semantics Sep 21, 2026
@ReubenBond ReubenBond changed the title fix(journaling): preserve failure and shutdown semantics fix(journaling): preserve first failure and handle owner shutdown Sep 21, 2026
Copilot AI review requested due to automatic review settings September 21, 2026 17:38
@ReubenBond ReubenBond changed the title fix(journaling): preserve first failure and handle owner shutdown fix(journaling): allow recovery retries and preserve persistence failures Sep 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The change has substantial lifecycle and recovery complexity, with mixed approval readiness and a documentation inconsistency remaining.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (1)

Comment thread src/Orleans.Journaling/IStateMachine.cs
Copilot AI review requested due to automatic review settings September 21, 2026 17:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Recovery retries must reset all registered state machines before replay to prevent stale in-memory data from being persisted.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread src/Orleans.Journaling/JournaledStateManager.cs Outdated
Copilot AI review requested due to automatic review settings September 21, 2026 20:53
@ReubenBond ReubenBond changed the title fix(journaling): allow recovery retries and preserve persistence failures fix(journaling): serialize recovery retries and preserve persistence failures Sep 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

A lifecycle test incorrectly expects an acknowledgement and append for a zero-byte write path.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread test/Orleans.Journaling.Tests/StateManagerLifecycleTests.cs
Copilot AI review requested due to automatic review settings September 21, 2026 21:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The lifecycle and recovery changes are broad and warrant final human review.

Review effort: Lite
Findings: None

Resolved since last review (1)

@ReubenBond
ReubenBond merged commit b3184eb into dotnet:main Sep 21, 2026
72 of 73 checks passed
@ReubenBond
ReubenBond deleted the rb-journaled-state-lifecycle branch September 21, 2026 21:48
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.

2 participants