fix(directory): deactivate recovery duplicates - #11030
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes distributed directory recovery and activation lifecycle behavior in the runtime, which warrants final human validation for correctness under rolling-upgrade/convergence scenarios.
Review tier: Lite
Findings: None
What changed in this PR
This PR addresses a rolling-upgrade recovery edge case in the distributed grain directory where multiple live activations for a single grain can remain visible after convergence, by tracking “losing” recovery observations and proactively deactivating the exact losing activation(s) before exposing the recovered directory range.
Changes:
- Update recovery entry selection to return the “losing” registration (when a duplicate is detected) while still preserving the newest membership-view winner.
- During partition-range recovery, collect duplicates, remove any that later become the winner, and deactivate remaining duplicates in bounded batches before completing recovery.
- Adjust
ICatalog.DeleteActivationsimplementation to deactivate only the exact activation address (identity-safe) and return immediately after making the loser unroutable; add/expand unit tests for the new recovery/cleanup behavior.
| File | Description |
|---|---|
| test/Orleans.GrainDirectory.Tests/GrainDirectory/GrainDirectoryPartitionTests.cs | Expands recovery tests to assert duplicate reporting, winner re-observation handling, and membership-version refresh for same activation identity. |
| src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs | Tracks recovery duplicates, removes re-observed winners from cleanup, and deactivates remaining duplicate activations in batches before exposing recovered ranges. |
| src/Orleans.Runtime/Catalog/Catalog.cs | Makes duplicate cleanup identity-safe by deactivating only when the local activation address matches exactly, and returns without awaiting full teardown. |
💡 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: mixed. 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 |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes core directory recovery and activation deactivation semantics in a way that can affect rolling-upgrade convergence behavior across silos, warranting final human validation.
Review tier: Lite
Findings: None
a51ff17 to
c133649
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes distributed recovery behavior and activation deactivation semantics in a way that’s correctness-critical under rolling-upgrade conditions and warrants final human review.
Review tier: Lite
Findings: None
Distributed grain-directory recovery can observe multiple live activations for one grain during the final rolling-upgrade transition. Recovery selected the registration from the newest membership view, but the losing activation remained valid after directory membership and range ownership converged.
This change records registrations which lose recovery selection and marks each exact losing activation deactivating before the recovered range is exposed. Cleanup is identity-safe, bounded into message-sized batches, preserves the newest observation of a repeated activation identity, and removes any activation which later becomes the final winner from the cleanup set. Full teardown and identity-conditional deregistration continue after the activation becomes unroutable, so recovery does not hold the range lock across grain deactivation callbacks.
This makes directory convergence include activation uniqueness while preserving normal deactivation and legacy-directory cleanup behavior.
Fixes #10998
Microsoft Reviewers: Open in CodeFlow