Skip to content

fix(runtime): preserve rebalancer state on shutdown - #10573

Merged
ReubenBond merged 2 commits into
dotnet:mainfrom
ReubenBond:rb-fix-migration-dead-silo-race
Aug 13, 2026
Merged

ReubenBond merged 2 commits into
dotnet:mainfrom
ReubenBond:rb-fix-migration-dead-silo-race

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Aug 13, 2026 •

Copy link
Copy Markdown
Member

Problem

During graceful silo shutdown, the activation rebalancer monitor requested migration of its worker but did not wait for the asynchronous migration to finish. Ordinary grain deactivation could therefore overtake the handoff, causing the worker to reactivate without its serialized state and leaving tests dependent on stale monitor reports and timing.

Solution

Move the rebalancer handoff to a lifecycle stage immediately before ordinary grain deactivation and await the worker's deactivation/migration completion. Update the regression test to synchronize initial placement and rebalancing cycles explicitly, verify the dead host is no longer used, and compare the preserved report state after migration. Placement hints are now cleared after test activation creation so they cannot leak into later calls.

Rationale

The migration manager and grain directory are still available at this stage, while the catalog has not yet begun bulk deactivation. Awaiting the handoff makes graceful shutdown ordering deterministic and preserves the worker's migration state instead of relying on retries or longer delays.

Fixes #10570

Microsoft Reviewers: Open in CodeFlow

Migrate and await the activation rebalancer worker before ordinary grain deactivation begins during graceful silo shutdown. Synchronize the regression test on migration and cycle completion and verify preserved worker state.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 13, 2026 04:21

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

This PR makes activation rebalancer shutdown deterministic by migrating the rebalancer worker earlier in the silo shutdown lifecycle and awaiting the worker’s deactivation/migration so its serialized state is preserved, addressing a flaky rebalancing state preservation test.

Changes:

  • Move rebalancer migration to a lifecycle stage immediately before ordinary grain deactivation and await the worker activation’s deactivation.
  • Update the state-preservation regression test to use event-driven synchronization for rebalancing cycles and validate report/state preservation post-migration.
  • Ensure placement hint RequestContext is always cleared after test activation creation to prevent leakage between calls.
Show a summary per file
File Description
test/Orleans.Placement.Tests/ActivationRebalancingTests/StatePreservationRebalancingTests.cs Refactors the regression test to coordinate on diagnostic events, validates migration/state preservation, and introduces helper for deterministic rebalancer relocation.
test/Orleans.Placement.Tests/ActivationRebalancingTests/RebalancingTestBase.cs Wraps placement hint usage in try/finally to avoid RequestContext leakage during test activation creation.
src/Orleans.Runtime/Placement/Rebalancing/ActivationRebalancerMonitor.cs Adds an earlier shutdown hook to migrate the rebalancer worker and await completion before bulk grain deactivation proceeds.

Review details

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

Suppressed comments (1)

test/Orleans.Placement.Tests/ActivationRebalancingTests/StatePreservationRebalancingTests.cs:145

  • MigrateOnIdle() initiates migration but does not wait for the source activation to deactivate/migrate. The immediate GetReport()/Assert.Equal can therefore race and intermittently observe the old host. Capture the current activation’s deactivation task before requesting migration and await it (similar to TestCluster.MigrateAsync/WaitForDeactivationAsync).
        RequestContext.Set(IPlacementDirector.PlacementHintKey, targetHost);
        try
        {
            await rebalancer.Cast<IGrainManagementExtension>().MigrateOnIdle();
        }
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Clear previously observed cycle events after the test load is created so the initial barrier only counts cycles which process that load.

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

Copilot-Session: 8fc23ff9-c5c7-435c-91c5-e23f9b6dc452
Copilot AI review requested due to automatic review settings August 13, 2026 14:09

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

Suppressed comments (1)

test/Orleans.Placement.Tests/ActivationRebalancingTests/StatePreservationRebalancingTests.cs:152

  • MigrateOnIdle() only initiates migration (it calls Migrate(...) without awaiting deactivation/rehydration), so immediately asserting the new host here can reintroduce timing-based flakiness. Consider waiting (with a bounded timeout) until GetReport().Host actually reflects targetHost, and perform the assert on the final attempt for a clearer failure message.
        Assert.Equal(targetHost, (await rebalancer.GetReport()).Host);
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ReubenBond
ReubenBond merged commit fca799f into dotnet:main Aug 13, 2026
73 checks passed
@ReubenBond
ReubenBond deleted the rb-fix-migration-dead-silo-race branch August 13, 2026 18:34
This was referenced Aug 28, 2026
This was referenced Sep 4, 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.

Flaky test: state-preserving migration does not leave dead silo

2 participants