feat(runtime): add view-synchronous cluster service coordination - #10969
ReubenBond wants to merge 20 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
DirectoryMembershipService.RefreshViewAsync currently fires-and-forgets an async ValueTask (_membership.RefreshViewAsync(...)), which can fault and become an unobserved exception, and should be replaced with an underlying refresh trigger (or awaited/observed).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Orleans.Runtime/GrainDirectory/DirectoryMembershipService.cs — RefreshViewAsync starts _membership.RefreshViewAsync(...) and discards the returned… |
What changed in this PR
This PR introduces reusable, view-synchronous “cluster service” primitives (configuration/view identity, deterministic topology projection, membership projection, operation results, and partition transition coordination) and refactors the distributed grain directory to use them, aiming to preserve the directory’s existing consistency boundaries while making the mechanisms reusable for other cluster-wide services.
Changes:
- Added
Orleans.Runtime.ClusterServicesprimitives (configuration/view ids, topology, membership projection, transition coordinator, and operation result). - Refactored grain directory membership/topology computation and range gating to use the new topology + transition coordinator abstractions.
- Added extensive internal test coverage (xUnit + CsCheck property tests + Microsoft.Accordant model-based tests) for topology determinism and transition protocols.
| File | Description |
|---|---|
| test/Orleans.Runtime.Internal.Tests/Orleans.Runtime.Internal.Tests.csproj | Adds Accordant dependency for model-based testing. |
| test/Orleans.Runtime.Internal.Tests/ClusterServices/PartitionTransitionCoordinatorTests.cs | Unit/property tests for transition coordinator gating and lifecycle semantics. |
| test/Orleans.Runtime.Internal.Tests/ClusterServices/ClusterServiceTopologyTests.cs | Property tests for deterministic topology projection and single-owner invariants. |
| test/Orleans.Runtime.Internal.Tests/ClusterServices/ClusterServiceOperationResultTests.cs | Contract tests for retry/disposition behavior. |
| test/Orleans.Runtime.Internal.Tests/ClusterServices/ClusterServiceMembershipTests.cs | Tests membership projection behavior (ordering, duplicates/regressions, refresh, disposal). |
| test/Orleans.Runtime.Internal.Tests/ClusterServices/ClusterServiceConfigurationTests.cs | Tests configuration validation and fingerprint determinism/culture invariance. |
| test/Orleans.Runtime.Internal.Tests/ClusterServices/ClusterServiceAccordantTests.cs | Accordant model-based tests for transition state machine coverage and rejection stability. |
| src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs | Replaces ad-hoc range locks with PartitionTransitionCoordinator and explicit transition staging/fencing. |
| src/Orleans.Runtime/GrainDirectory/DistributedGrainDirectory.cs | Adds fatal error handling hook for membership update task failures and exposes snapshot for test hooks. |
| src/Orleans.Runtime/GrainDirectory/DirectoryMembershipSnapshot.cs | Refactors directory membership snapshot to wrap ClusterServiceTopology and reuse its projection. |
| src/Orleans.Runtime/GrainDirectory/DirectoryMembershipService.cs | Replaces direct snapshot projection with ClusterServiceMembership + view update publishing. |
| src/Orleans.Runtime/ClusterServices/PartitionTransitionCoordinator.cs | New transition coordinator + transition state machine for per-range view-synchronous gating. |
| src/Orleans.Runtime/ClusterServices/ClusterServiceTopology.cs | New deterministic ring/topology projection + ownership/range queries. |
| src/Orleans.Runtime/ClusterServices/ClusterServiceOperationResult.cs | New execution disposition/retry result type for cluster-service operations. |
| src/Orleans.Runtime/ClusterServices/ClusterServiceMembership.cs | New membership projection wrapper with ordered view updates + refresh coordination. |
| src/Orleans.Runtime/ClusterServices/ClusterServiceConfiguration.cs | New config + fingerprinting + view-id direct-successor logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It makes deep, correctness-critical changes to distributed grain directory membership/transition coordination, so it warrants final human validation of the new consistency boundaries and failure modes.
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Runtime/GrainDirectory/DirectoryMembershipService.cs — RefreshViewAsync starts _membership.RefreshViewAsync(...) and discards the returned… View resolved comment |
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Orleans.Runtime/ClusterServices/ClusterServiceMembership.cs:57
- The version gate check uses
<=, which causesRefreshViewAsyncto enter the async enumeration path even whenCurrentViewis already exactly at the requested minimum version. This adds avoidable allocations (linked CTS, async enumerator) on a potentially hot path.
Switching the condition to a strict < keeps semantics the same (still waits when the view is behind) while avoiding the extra async work when the boundary is already satisfied.
Code coverage
Report-only conclusion: current-main baseline stale. The newest successful coverage run tested 114a827, not current main 9cf2fac. 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 |
6cdcc00 to
c5ec951
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A new [GenerateSerializer] type (ClusterServiceOperationResult<T>) currently has a private constructor with get-only properties, which can prevent Orleans’ generated serializers from constructing it during deserialization.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Orleans.Runtime/ClusterServices/ClusterServiceOperationResult.cs — ClusterServiceOperationResult<T> is marked with [GenerateSerializer] but has only get-only… |
c5ec951 to
742e569
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It makes substantial, correctness-critical changes to grain directory membership/view gating and introduces new coordination primitives whose failure modes warrant careful human validation.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs — EmitRangeOperationCompleted(..., canceled) is now passed canceled || !canComplete, which… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Runtime/ClusterServices/ClusterServiceOperationResult.cs — ClusterServiceOperationResult<T> is marked with [GenerateSerializer] but has only get-only… View resolved comment |
7bbf8dd to
6a6843e
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The changes significantly restructure grain-directory membership/topology/transition coordination and introduce new correctness-critical primitives, warranting careful human validation of edge cases and compatibility expectations beyond what can be safely concluded from this review context.
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs — EmitRangeOperationCompleted(..., canceled) is now passed canceled || !canComplete, which… View resolved comment |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It makes broad, correctness-critical changes to distributed grain directory membership/transition gating and introduces new coordination primitives, warranting final human review despite strong test coverage.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Orleans.Runtime/ClusterServices/ClusterServiceMembership.cs — In RefreshViewAsync, the condition CurrentView.ViewId.MembershipVersion <= minimumVersion… |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The changes introduce new core coordination primitives and refactor grain-directory membership/transition behavior, which is runtime-critical and warrants final human review despite strong test coverage.
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Runtime/ClusterServices/ClusterServiceMembership.cs — In RefreshViewAsync, the condition CurrentView.ViewId.MembershipVersion <= minimumVersion… View resolved comment |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It introduces new coordination primitives and substantially refactors the distributed grain directory’s membership/ownership-transition pipeline, which is core runtime behavior and warrants final human validation.
Review tier: Lite
Findings: None
f576d55 to
a183e2a
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Prevent duplicate compatibility commands from executing and correct the failure-completion documentation.
Get a fresh assessment by requesting another Copilot review.
Review tier: Lite
Findings: 1
Open findings (2)
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three unresolved findings remain, including one critical compatibility-host issue and two moderate view-refresh/thread-safety issues.
Get a fresh assessment by requesting another Copilot review.
Review tier: Lite
Findings: 1
Open (1)
Resolved since last review (2)
| <TargetFrameworks>$(TestTargetFrameworks)</TargetFrameworks> | ||
| <OutputType>Exe</OutputType> | ||
| <Nullable>enable</Nullable> | ||
| <AssemblyName>Orleans.GrainDirectory.Compatibility.ReleaseHost</AssemblyName> |
Exercise the default in controlled and mixed-version scenarios, update guidance, and bound ElasticChaos with an overall deadline and independent cleanup limits.
Use the runtime output layout instead of CallerFilePath, which deterministic CI builds map to a virtual source path.
Restore minimum-view admission and the original directory RPC signatures. Preserve liveness, recovery, harness, and CI fixes, and document admission policy as a service-specific choice.
Cover duplicate stop IDs in both compatibility hosts and correct the documentation of faulted transition completion.
4855683 to
b8f806e
Compare
Problem
Cluster-wide services need the ownership, state-continuity, and fencing boundaries used by the distributed grain directory, whose topology and transition mechanisms were tightly coupled to its implementation.
Solution
Rationale
These primitives provide view-synchronous primary ownership: operations pass ordered view boundaries, successors install state and establish fencing before serving, and failed transitions preserve the recovery boundary. Admission policy belongs to each service's operation contract. The grain directory uses minimum-view synchronization; other integrations can select policies justified by their state, configuration, and fencing requirements.
Register-backed assignment and configuration are follow-up work. A reminder-service integration also requires an authoritative ownership-epoch fence or an equivalent atomic scan-and-fence contract from its providers.
Microsoft Reviewers: Open in CodeFlow