Skip to content

fix(membership): preserve canonical membership views - #11296

Merged
ReubenBond merged 8 commits into
dotnet:mainfrom
ReubenBond:rb-fix-membership-snapshot-invariants
Sep 17, 2026
Merged

ReubenBond merged 8 commits into
dotnet:mainfrom
ReubenBond:rb-fix-membership-snapshot-invariants

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

The membership system provides monotonically versioned canonical membership views. Per-silo IAmAliveTime advances independently of the view version, and local snapshots can compact rows which were already Dead.

This change gives table reads and peer snapshots one merge rule: preserve accepted versioned fields at the same version, take versioned fields from newer views, and retain the maximum observed IAmAliveTime. Successor detection relies on those canonical-field guarantees. A same-version snapshot can advance liveness or prune previously Dead rows while retaining every non-Dead row. Adding or replacing a row requires a newer version. Peer-snapshot application observes caller cancellation before publication.

Losing the development primary's in-memory table invalidates the cluster. SystemTargetBasedMembershipTable directly invokes IFatalErrorHandler and rejects a row or full-table read whose version is below the membership version known before the read began. Capturing that version before the read preserves correct handling of overlapping reads. Ordinary gossip and shutdown behavior remain in place.

Focused regressions cover same-version Dead-only pruning, retained canonical fields, maximum liveness timestamps, cancellation, and fatal rollback. Provider-level versioned compaction and its conformance suite remain separate work; this PR specifies snapshot acceptance. Documentation records the snapshot and cluster-lifetime guarantees. This is a main-based prerequisite for #10236.

Copilot AI lite review requested due to automatic review settings September 17, 2026 07:49

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

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Preserves membership snapshot invariants across provider reads, cleanup, development-table resets, and gossip cancellation.

Changes:

  • Validates same-version membership fields while allowing heartbeat advancement and inactive cleanup.
  • Adds development-provider refresh and version rollback handling.
  • Expands regression coverage for snapshots, resets, and cancellation.
File Description
test/​Orleans.Core.Tests/​Membership/​MembershipTableSnapshotTests.cs Updated as part of this pull request.
test/​Orleans.Core.Tests/​Membership/​MembershipTableManagerTests.cs Updated as part of this pull request.
src/​Orleans.Runtime/​MembershipService/​MembershipTableManager.cs Updated as part of this pull request.
src/​Orleans.Core/​SystemTargetInterfaces/​IMembershipTable.cs Updated as part of this pull request.
src/​Orleans.Core/​Runtime/​MembershipTableSnapshot.cs Updated as part of this pull request.

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

Comment thread src/Orleans.Runtime/MembershipService/MembershipTableManager.cs Outdated
@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request Current main Variance
Lines 82.22% (112,484 / 136,814) 82.21% (112,436 / 136,768) +0.0074 pp
Branches 71.46% (32,431 / 45,385) 71.46% (32,419 / 45,369) +0.0012 pp

Report-only conclusion: improved.

The current-main baseline is commit 9303d4ec7c 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 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 17, 2026 15:29

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 17, 2026 17:01

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 issues were identified that would block approval.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 17, 2026 20:35

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

The changes are covered by focused regression tests and preserve the documented membership ownership and cancellation semantics.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 17, 2026 21:13
@ReubenBond ReubenBond changed the title fix(membership): preserve authoritative snapshot invariants fix(membership): preserve canonical membership views Sep 17, 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

The changes and regression coverage consistently implement the documented canonical-view and rollback guarantees.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 17, 2026 21: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.

Copilot review overview

🔵 Needs a closer look

Successor detection can accept a snapshot that regresses an observed liveness timestamp.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 17, 2026 21: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

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Review effort: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 17, 2026 21:27

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 issues were identified that would block approval.

Review effort: Lite
Findings: None

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