Skip to content

feat(journaling): manage independently owned dictionary values - #11469

Closed
ReubenBond wants to merge 2 commits into
dotnet:mainfrom
ReubenBond:rb-durable-dictionary-ownership
Closed

ReubenBond wants to merge 2 commits into
dotnet:mainfrom
ReubenBond:rb-durable-dictionary-ownership

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Problem

Durable dictionary values can own explicitly retained resources. Each stored entry needs an independent owner so callers can release their own values and replacement, removal, recovery, and teardown can retire the correct resources.

Solution

Add IDurableDictionaryValueLifecycle<TValue> and resolve an optional registered lifecycle when constructing a durable dictionary. Live mutations acquire an independent owner before encoding; failed encoding releases that acquired owner. Replay transfers decoded owners directly. Replacement, removal, clear, reset, journal deletion, and dictionary disposal release stored owners. Dictionary reads borrow their values; retention preserves value contents and equality semantics.

JSON dictionary snapshots validate pair structure and the required key before materializing the value. This closes an ownership gap where malformed snapshot pairs could deserialize a resource and throw before transferring it to the dictionary. The validation uses a copied reader and a structural traversal, with no owner-tracking registry or per-entry state. Ordinary unregistered assignments keep a single dictionary insertion and bypass the ownership-only previous-value lookup and cleanup catch.

Generic resource-probe tests cover aliases under multiple keys, self-replacement, equal borrowed-pair removal, rejected mutations, actual binary and default-JSON journal persistence/replay, failed recovery, recovery retry, snapshot rejection before owner acquisition, journal deletion, activation-scope disposal, and standalone caller-owned component lifetime. The generated API and focused ownership documentation accompany the implementation.

Scope and rationale

Rebased onto dotnet/orleans main b3b76e2ab207f00e664e0167cd733800cd60379d and reviewed against the updated AGENTS.md and repository code-review skill. This standalone change relies on the manager's existing named-state deletion/reset and DI/caller disposal guarantees. It has no prerequisite PR dependency and preserves the existing V1 framing marker, V0 reader, and normal released-package compatibility.

The automated coverage comment for the previous head reported 83.40% combined line coverage and an improvement over main; its BVT artifact covered 138/147 dictionary implementation lines. Updated-head CI coverage is pending. Structural costs are explicit: one lifecycle reference and disposal flag per dictionary, no added entry metadata, and one extra traversal per JSON snapshot pair for pre-materialization validation. Throughput and object-size gains have not been benchmarked.

Microsoft Reviewers: Open in CodeFlow

Copilot AI balanced review requested due to automatic review settings October 10, 2026 16:22

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.

🟢 Approval recommended

The ownership behavior is consistently implemented, documented, and covered by focused lifecycle and persistence tests.

0 open findings

What changed in this PR

Adds independent ownership management for resource-backed durable dictionary values.

Changes:

  • Introduces a configurable value lifecycle for retain/release operations.
  • Releases owned values during mutation, replay, reset, deletion, and disposal.
  • Adds ownership documentation, generated API updates, and comprehensive tests.
File Description
src/​Orleans.Journaling/​DurableDictionary.cs Integrates value ownership into dictionary operations.
src/​Orleans.Journaling/​IDurableDictionaryValueLifecycle.cs Defines the public lifecycle contract.
src/​api/​Orleans.Journaling/​Orleans.Journaling.cs Updates the generated API surface.
src/​Orleans.Journaling/​README.md Documents ownership semantics.
docs/​site/​src/​content/​docs/​grains/​journaling/​durable-state.md Adds user guidance for owned values.
test/​Orleans.Journaling.Tests/​OwnedDictionaryValueTests.cs Covers retention, release, replay, failure, and disposal behavior.

🧠 Review effort: Balanced


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

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request Current main Variance
Lines 83.38% (118,402 / 142,007) 83.39% (118,367 / 141,948) -0.0100 pp
Branches 72.91% (35,177 / 48,246) 72.94% (35,177 / 48,226) -0.0302 pp

Report-only conclusion: regressed.

The current-main baseline is commit b3b76e2ab2 and uses the same reviewed coverage matrix.

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 balanced review requested due to automatic review settings October 10, 2026 19:18
@ReubenBond
ReubenBond force-pushed the rb-durable-dictionary-ownership branch from 988eec2 to 3161e20 Compare October 10, 2026 19:18

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.

🟡 Changes recommended

Replay can leak transferred owners after disposal, and the documented caller-owned disposal path is not publicly available.

3 open findings

🧠 Review effort: Balanced

Comment on lines +17 to +18
/// release must complete without throwing. The activation scope disposes DI-created dictionaries.
/// Callers arrange disposal of manually constructed dictionaries and their dependencies.

Replay transfers decoded owners into the dictionary. Replacement, successful removal, clear, reset, and whole-journal deletion release the stored owners. A rejected mutation releases any owner it acquired and preserves existing entries. Snapshot encoding borrows the current entries.

Lifecycle implementations preserve value contents and equality when retaining an owner, complete retention atomically, and release synchronously without throwing. Callbacks operate on the supplied value's resources, with dictionary mutation left to the caller. Command codecs borrow values during encoding and transfer decoded owners to the dictionary. Orleans disposes DI-created dictionaries with the activation scope, releasing their remaining owners even when recovery fails partway through a journal. Applications arrange disposal of manually constructed dictionaries and their dependencies. Value types with ordinary managed lifetimes retain the standard dictionary reference semantics.
Comment on lines +175 to +177
dictionary disposal release stored owners. The activation scope disposes DI-created dictionaries;
callers arrange disposal of manually constructed components. Lifecycle implementations retain
atomically and release synchronously without throwing.
@ReubenBond

Copy link
Copy Markdown
Member Author

Deferring this feature now that Durable Messaging uses GC-owned byte[] envelopes. There is no identified consumer requiring independently owned dictionary values, so the public lifecycle contract and its mutation/replay/recovery/disposal complexity are not justified at present. The ownership-specific JSON changes are deferred with this PR. Preserving the branch so a concrete future consumer can inform a smaller design.

@ReubenBond ReubenBond closed this Oct 10, 2026
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