Multi-instance safety: Redis cache invalidation resilience & exactly-once scheduled tasks#731
Open
KrzysztofPajak wants to merge 6 commits into
Open
Multi-instance safety: Redis cache invalidation resilience & exactly-once scheduled tasks#731KrzysztofPajak wants to merge 6 commits into
KrzysztofPajak wants to merge 6 commits into
Conversation
…eployments RedisMessageBus (cache sync between instances): - clear local cache when the Redis subscription connection is restored - pub/sub has no replay, so invalidations published while disconnected are lost and the local cache can no longer be trusted - subscribe with retry (exponential backoff) instead of a discarded task - replace Debug.WriteLine with ILogger; log publish delivery (receiver count) and received invalidations at Debug level - await message publication in RedisMessageCacheManager - connect with AbortOnConnectFail=false and register IConnectionMultiplexer; remove duplicated ICacheBase registration Scheduled tasks (no duplicated runs across instances): - add ScheduleTaskService.TryClaimTaskRun - atomic compare-and-set on LastStartUtc, so exactly one instance wins a run (fixes e.g. duplicated queued e-mails); losing instance skips and retries next interval - persist the task entity only when the instance actually ran it, so a losing instance no longer overwrites the winner's results with stale data - identify instances by a per-process GUID (pod/machine names are not stable) Aspire: run two Grand.Web instances with Redis pub/sub enabled to exercise the multi-instance scenario locally; disable plugin shadow copy (instances share one content root). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…t, rethrow cancellation on shutdown - ScheduleTaskService.TryClaimTaskRun: explicit task null check instead of null-conditional chain - BackgroundServiceTask: remove unread runTask assignment; rethrow OperationCanceledException during shutdown so an interrupted run is not recorded as a task failure Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…structor Task.Run Starting a fire-and-forget Task.Run from the constructor did work in the ctor, left the task unobserved and outside any lifecycle. RedisMessageBus now implements IHostedService: the subscription retry loop starts in StartAsync (tracked in a field) and is cancelled and awaited in StopAsync; connection event handlers are attached/detached with the same lifecycle. Registered as a single instance exposed both as IMessageBus and as a hosted service. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two separate AddProject resources for the same project could crash a Grand.Web instance at startup with host error 0x80008081 while both raced over the shared project directory. Use a single AddProject with WithReplicas(2) so Aspire launches two processes of one built project; each replica gets its own port assigned by Aspire (the fixed WithHttpEndpoint is dropped, as replicas cannot share a port). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- BackgroundServiceTask: use a structured logging template instead of string interpolation so the message is only formatted when the log level is enabled (S2629) - RedisMessageBus: reword a prose comment that S125 mistook for commented-out code Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Debug/Information log calls in RedisMessageBus and BackgroundServiceTask pass arguments (enum casts, properties) that are evaluated even when the level is disabled. Guard them with ILogger.IsEnabled so the arguments are only evaluated when the message is actually emitted - a real saving on the cache publish/receive hot paths where Debug is off in production. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
When running multiple instances (Kubernetes pods, replicas):
RedisMessageBuswas best-effort only. Redis pub/sub has no replay - invalidations published while an instance was disconnected were silently lost, and the instance kept serving stale data until cache TTL. Publish/subscribe failures were swallowed withDebug.WriteLine, invisible in production.LastStartUtc, so two instances racing at the same interval both executed the task - e.g. queued e-mails were sent twice.LeasedByMachineNamedid not help (manual pin, empty by default, pod names rotate).Changes
RedisMessageBus / cache
ConnectionRestored(subscription connection) the local cache is cleared withClear(false)- invalidations missed while disconnected make it untrustworthy;ConnectionFailedis logged as a warningILoggerinstead ofDebug.WriteLine; failed publish logs an error, successful publish/receive log at Debug level (publish logs the subscriber count returned by Redis - handy for verifying instances actually listen)RedisMessageCacheManagerawaits publication; duplicated removal logic delegated toMemoryCacheBaseAbortOnConnectFail=false(no crash-loop when Redis is briefly unavailable at pod start),IConnectionMultiplexerregistered for connection events, duplicatedICacheBaseregistration removedScheduled tasks - exactly-once per interval
IScheduleTaskService.TryClaimTaskRun: atomic compare-and-set onLastStartUtc(conditionalUpdateOneAsync) + read-back verification via newScheduleTask.LeasedByInstancefield; exactly one instance wins each run, others skip and re-check next interval (natural failover when the winning pod dies)BackgroundServiceTaskexecutes only after winning the claim; instance identity is a per-process GUIDUpdateTasklet a losing instance overwrite the winner's results with stale dataLeasedByMachineNamepin behaviour unchanged; note: LiteDB'sUpdateOneAsyncis find-then-update (non-atomic), but LiteDB is a single-instance database anywayAspire (local multi-instance testing)
Grand.Webinstances (ports 80 / 8080) + Redis container with pub/sub enabled, Debug logging for the cache message bus, plugin shadow copy disabled (instances share one content root)Tests
RedisMessageBusTests(8): publish payload/channel, error swallowing, subscription on startup, dispatch of received messages withpublisher=false, cache clear on restored subscription connection (and not on interactive)RedisMessageCacheManagerTests(6): publish per operation type, no publish withpublisher=falseScheduleTaskServiceTests(4, MongoDB integration): first claim wins, stale claim fails and keeps winner, concurrent claims - exactly one winner, missing taskBackgroundServiceTaskTests(4): claim won → execute + persist; claim lost → no execute, no stale persist; not due → no claim; machine pin respectedAll affected suites green: Grand.Infrastructure.Tests 83/83, Grand.Modules.Tests 18/18, Grand.Web.Common.Tests 15/15.
🤖 Generated with Claude Code