fix(directory): support rolling upgrades to distributed directory - #10309
ReubenBond merged 4 commits into
Conversation
Add a three-silo rolling-upgrade test with sustained traffic and diagnostics, preserve targeted cache invalidation during shutdown, and bound recovery calls using shared per-member cancellation tokens. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0be5952a-8f00-4f60-a0dd-4fbc8c0ebe7d
There was a problem hiding this comment.
Pull request overview
This PR improves Orleans’ ability to roll from LocalGrainDirectory to DistributedGrainDirectory in live clusters by adding a sustained rolling-upgrade test suite and refining shutdown/rejection behaviors to prevent stalls, stale routing, and disruptive cache resets.
Changes:
- Added an end-to-end three-silo rolling upgrade test which maintains “hot” and “fresh” grain traffic while restarting silos in place and validating convergence/integrity.
- Updated shutdown-time messaging to reject inbound/outbound application requests with targeted cache invalidation while allowing responses to complete.
- Introduced per-member cancellation token reuse for distributed directory operations and added unit tests covering the token lifecycle semantics.
Show a summary per file
| File | Description |
|---|---|
| test/Orleans.GrainDirectory.Tests/GrainDirectory/GrainDirectoryRollingUpgradeTests.cs | Adds the rolling upgrade suite and supporting traffic/log/cache instrumentation. |
| test/Orleans.GrainDirectory.Tests/GrainDirectory/GrainDirectoryPartitionTests.cs | Adds unit tests validating ClusterMemberCancellationTokens behavior across membership transitions. |
| src/Orleans.Runtime/Networking/SiloConnection.cs | Changes shutdown-time inbound request handling to use targeted cache invalidation rejection logic. |
| src/Orleans.Runtime/Messaging/MessageCenter.cs | Adjusts shutdown-time outbound message blocking to allow responses while rejecting requests with cache invalidation; exposes ProcessRequestToInvalidActivation for reuse. |
| src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs | Bounds per-member operations using the reused cancellation token and avoids waiting indefinitely on departing members. |
| src/Orleans.Runtime/GrainDirectory/DistributedRemoteGrainDirectory.cs | Minor adjustment to timeout CTS creation to use the shared helper overload. |
| src/Orleans.Runtime/GrainDirectory/DistributedGrainDirectory.cs | Adds membership-driven per-member cancellation token management and publishes updates from the membership loop. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 2
Recheck refreshed membership before retrying departed members, reduce the test cache capacity, and narrowly classify expected shutdown routing errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0be5952a-8f00-4f60-a0dd-4fbc8c0ebe7d
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (1)
src/Orleans.Runtime/GrainDirectory/GrainDirectoryPartition.cs:792
InvokeOnClusterMemberawaitsoperationTask.WaitAsync(memberCancellationToken). SinceClusterMemberCancellationTokensdisposes per-memberCancellationTokenSourceinstances when a member becomes unavailable, there is a race whereWaitAsynccan throwObjectDisposedException(token source disposed) instead ofOperationCanceledException. That exception will currently fall through to the outercatch (Exception ex)and be logged/retried as an error, even though it should be treated like member cancellation.
Consider handling ObjectDisposedException in the same way as cancellation so the retry loop exits cleanly when the member token is torn down.
return await operationTask.WaitAsync(memberCancellationToken);
}
catch (OperationCanceledException) when (memberCancellationToken.IsCancellationRequested)
{
if (operationTask.IsCompleted)
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
Pass shared member cancellation tokens into directory system-target calls, remove per-call WaitAsync coordination, and use the member token as the retry-loop guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0be5952a-8f00-4f60-a0dd-4fbc8c0ebe7d
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (2)
src/Orleans.Runtime/Networking/SiloConnection.cs:100
- When blocking application messages, the invalidation address uses msg.TargetSilo, but TargetSilo can be null (see comment below in this method). If it is null here, the cache-invalidation header becomes ineffective and local cache invalidation via GrainLocator may also miss. Prefer falling back to the local silo address so the invalidation is always targeted at the stopping silo.
new GrainAddress { GrainId = msg.TargetGrain, SiloAddress = msg.TargetSilo },
src/Orleans.Runtime/Messaging/MessageCenter.cs:169
- Outbound blocked requests are currently rejected via ProcessRequestToInvalidActivation using an 'oldAddress' built from msg.TargetGrain/msg.TargetSilo. That adds a cache-invalidation header for the target grain/activation even though the target is not invalid—the send failed only because this silo is stopping—so it can cause spurious cache invalidations and extra directory lookups during shutdown. Prefer rejecting locally without cache invalidation (and include a SiloUnavailableException for diagnostics).
if (msg.Direction == Message.Directions.Request)
{
ProcessRequestToInvalidActivation(
msg,
new GrainAddress { GrainId = msg.TargetGrain, SiloAddress = msg.TargetSilo },
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
Treat GrainCallCancellationManager failures caused by intentionally restarting an unavailable silo as expected rolling-upgrade diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0be5952a-8f00-4f60-a0dd-4fbc8c0ebe7d
There was a problem hiding this comment.
Copilot's findings
Suppressed comments (3)
test/Orleans.GrainDirectory.Tests/GrainDirectory/GrainDirectoryRollingUpgradeTests.cs:1941
- Since TrackingGrainDirectoryCache now needs to be disposable (to avoid leaking the inner cache's expiration timer/task), add a DisposeAsync implementation which forwards disposal to the inner cache when supported.
public bool LookUp(GrainId key, out GrainAddress result, out int version) =>
_inner.LookUp(key, out result, out version);
}
src/Orleans.Runtime/Networking/SiloConnection.cs:105
- When the silo is blocking application messages, this branch currently drops all non-request messages, including responses. That conflicts with the intended "allow already-produced responses to complete" behavior and the MessageCenter.BlockApplicationMessages documentation (responses should be allowed while stopping). It also blocks membership-table traffic unless it is flagged as a system message. Consider allowing Response-direction messages (and membership table grain messages) to pass through the normal pipeline, while still rejecting only inbound requests and dropping other inbound one-way messages.
// If we've stopped application message processing, then reject requests and filter out other messages.
// Note that if we identify or add other grains that are required for proper stopping, we will need to treat them as we do the membership table grain here.
if (messageCenter.IsBlockingApplicationMessages && !msg.IsSystemMessage)
{
// We reject new requests with targeted cache invalidation and drop all other messages.
if (msg.Direction != Message.Directions.Request)
{
this.MessagingTrace.OnDropBlockedApplicationMessage(msg);
return;
}
messageCenter.ProcessRequestToInvalidActivation(
msg,
new GrainAddress { GrainId = msg.TargetGrain, SiloAddress = msg.TargetSilo },
forwardingAddress: null,
failedOperation: "Silo stopping",
rejectMessages: true);
return;
}
test/Orleans.GrainDirectory.Tests/GrainDirectory/GrainDirectoryRollingUpgradeTests.cs:1920
- TrackingGrainDirectoryCache wraps LruGrainDirectoryCache (which starts a PeriodicTimer-based expiration loop) but does not implement IDisposable/IAsyncDisposable, so the inner cache will not be disposed by DI when the silo host is torn down. This can leak timers/background tasks across test runs. Implement IAsyncDisposable (or IDisposable) on the wrapper so the inner cache is disposed with the host.
This issue also appears on line 1939 of the same file.
internal sealed class TrackingGrainDirectoryCache : IGrainDirectoryCache
{
private readonly IGrainDirectoryCache _inner = new LruGrainDirectoryCache(
maxCacheSize: 4_096,
maxCacheTTL: TimeSpan.FromMinutes(10),
timeProvider: TimeProvider.System);
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
Live clusters need to migrate from
LocalGrainDirectorytoDistributedGrainDirectorywithout directory stalls, stale routing, or disruptive cache resets during a rolling deployment.This change adds a realistic three-silo rolling-upgrade suite which keeps sustained hot and fresh grain traffic running while silos are restarted one at a time. It captures phase-aware membership, directory handoff, activation, cache, caller, and error diagnostics, and verifies mixed-mode convergence, unique activations, targeted invalidation, and the absence of whole-cache clears.
The runtime changes:
Task.WhenAnyShuttingDownas callable and cancel member operations only atStopping,Dead, or removalA bounded retry is used only for the unavoidable in-flight disruption when the client's gateway itself restarts. Persistent failures and all post-convergence failures remain test failures.
Microsoft Reviewers: Open in CodeFlow