Skip to content

refactor(runtime): centralize placement compatibility caching - #10599

Merged
ReubenBond merged 2 commits into
dotnet:mainfrom
ReubenBond:rb-refactor-placement-compatibility-caching
Aug 14, 2026
Merged

ReubenBond merged 2 commits into
dotnet:mainfrom
ReubenBond:rb-refactor-placement-compatibility-caching

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Aug 14, 2026 •

Copy link
Copy Markdown
Member

PlacementService maintained a second compatibility cache and its own invalidation plumbing alongside CachedVersionSelectorManager. That duplication made placement compatibility state harder to reason about and risked the two caches observing membership and manifest publication at different points.

This change centralizes placement compatibility caching in CachedVersionSelectorManager. PlacementService now delegates compatible-silo lookups to the selector manager, while the selector preserves the retry yielding and cache-clearing behavior introduced by #10597 and removes the temporary CacheInvalidated event and legacy constructor shim.

This follows #10596 and #10597, consolidating ownership of compatibility cache state so placement decisions align with the published membership and grain manifest versions.

Microsoft Reviewers: Open in CodeFlow

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8a704efe-ccce-454f-b619-0f873bcdcc1e

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.

Pull request overview

Centralizes placement compatibility caching in CachedVersionSelectorManager so placement decisions are aligned with membership + cluster manifest publication generations, removing duplicated caching/invalidation logic from PlacementService.

Changes:

  • Removed PlacementService’s local compatible-silos cache/invalidation plumbing and delegated compatible-silo lookups to CachedVersionSelectorManager.
  • Simplified CachedVersionSelectorManager by requiring IClusterMembershipService and removing the temporary CacheInvalidated event path.
  • Updated PlacementServiceTests to construct a real CachedVersionSelectorManager (with membership) and adjusted coverage for shutdown/membership/manifest scenarios.
Show a summary per file
File Description
test/Orleans.Core.Tests/Runtime/PlacementServiceTests.cs Updates test fixture to use membership-backed CachedVersionSelectorManager and adjusts/extends compatibility caching tests.
src/Orleans.Runtime/Versions/CachedVersionSelectorManager.cs Requires IClusterMembershipService and removes the cache invalidation event shim.
src/Orleans.Runtime/Placement/PlacementService.cs Removes duplicated compatibility cache and delegates compatibility queries to the selector manager.

Review details

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread test/Orleans.Core.Tests/Runtime/PlacementServiceTests.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 3afaf993-416a-46da-b8c3-8e44292d917f
Copilot AI review requested due to automatic review settings August 14, 2026 21: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.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ReubenBond
ReubenBond merged commit b191ded into dotnet:main Aug 14, 2026
135 of 137 checks passed
@ReubenBond
ReubenBond deleted the rb-refactor-placement-compatibility-caching branch August 14, 2026 23:52
This was referenced Sep 7, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants