fix(directory): preserve newer recovery registrations - #10533
Merged
ReubenBond merged 4 commits intoAug 12, 2026
Merged
Conversation
Keep partition recovery responses from replacing registrations created in a newer membership view, regardless of response order. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a rolling-upgrade recovery correctness issue in the distributed grain directory: when merging recovered activation registrations, response ordering could previously regress directory state. The fix makes recovery prefer registrations from the highest cluster membership version and adds a deterministic unit test to cover both response orders.
Changes:
- Update partition recovery to merge recovered entries by choosing the highest
MembershipVersion(instead of last-writer-wins by response order). - Introduce
GrainDirectoryPartition.RecoverEntry(...)to centralize recovery-merge behavior. - Add a unit test to validate recovery outcome is independent of response ordering.
Show a summary per file
| File | Description |
|---|---|
| src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs | Changes recovery merge logic to prefer higher MembershipVersion via RecoverEntry. |
| test/Orleans.GrainDirectory.Tests/GrainDirectory/GrainDirectoryPartitionTests.cs | Adds deterministic coverage for recovery merge behavior across both response orders. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs:792
LogTraceRecoveredEntryruns beforeRecoverEntry, so trace logs will say an entry was “Recovered” even whenRecoverEntryignores it due to an existing entry with a higherMembershipVersion. That can make recovery troubleshooting misleading. Consider logging only when the recovered entry is actually applied to_directory.
DebugAssertOwnership(current, entry.GrainId);
LogTraceRecoveredEntry(_logger, entry, current.Version);
RecoverEntry(_directory, entry);
}
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Explain why mixed directory recovery can observe superseded activations and that same-version conflicts remain invalid. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fec7e7f-c494-411f-ab29-08fca4d18191
Clarify that cross-version activation conflicts are expected only while LocalGrainDirectory and DistributedGrainDirectory coexist, without changing same-view tie-breaking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4fec7e7f-c494-411f-ab29-08fca4d18191
This was referenced Aug 20, 2026
This was referenced Aug 28, 2026
Merged
This was referenced Aug 31, 2026
Merged
This was referenced Sep 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During partition recovery, activation responses from live silos were merged using last-writer-wins ordering. In a rolling upgrade, a response from an older LocalGrainDirectory activation could therefore overwrite a registration created in a newer distributed-directory membership view, leaving the recovered directory pointing at the superseded activation.
Prefer recovered registrations with the highest membership version so response order cannot regress directory state. Add deterministic coverage for both response orders.
Fixes #10519