feat(api): Redis distributed cache + durable background job queue (#217 #218) - #214
Conversation
…217) Move the user-date-preferences cache in UserDateService off the per-instance IMemoryCache and onto the framework IDistributedCache seam, so a second API instance never serves a stale timezone or week-start preference. The backing store is selected by a rollout flag: with Redis disabled (default) an in-process IDistributedCache is registered and behavior is unchanged; with it enabled the same seam is backed by Redis (Render Key-Value). Preferences serialize as JSON with the existing 15-minute TTL, and a preference change still invalidates the entry. Only the user-prefs cache moves; the AI-content IMemoryCache caches are unrelated and stay as-is. Closes thomasluizon/orbit-ui-mobile#217 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a durable, flag-gated path for the recurring background schedulers so a restart no longer drops in-flight work, two instances no longer double-run a scan, and a transient failure retries with backoff instead of waiting a full interval. With BackgroundServices:UseDurableQueue off (default) every scheduler runs as its existing in-process BackgroundService polling loop, unchanged. With it on, the ten recurring scans are registered as Hangfire recurring jobs backed by PostgreSQL (reusing ConnectionStrings:DefaultConnection, separate Hangfire schema): occurrences persist across restarts, Hangfire's distributed lock keeps a single instance per occurrence, and failed runs retry with exponential backoff. The in-process loops are not registered in that mode, so the two paths never run at once. Each recurring scheduler now implements IScheduledJob (name + cron + RunAsync) delegating to its existing scan; a single ScheduledJobRunner is the one Hangfire entry point (storage persists only the job name), and HangfireRecurringJobRegistrar reconciles the schedule on startup. The one-shot DataEncryptionMigrationService stays a hosted service in both modes. PostgreSQL was chosen over Redis for the queue store to keep job state transactional with the domain data already in Postgres and avoid a second durability-critical dependency. Storage/queue decision and chosen backing store are documented in the PR. Also carries the #217 distributed-cache DI/config wiring in the shared bootstrap files (ServiceCollectionExtensions, appsettings, csproj). Closes thomasluizon/orbit-ui-mobile#218 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
||
| public string CronExpression => "* * * * *"; | ||
|
|
||
| public Task RunAsync(CancellationToken cancellationToken) => CheckAndSendReminders(cancellationToken); |
There was a problem hiding this comment.
RecordTick is never called from RunAsync — BackgroundServiceHealthCheck permanently reports Healthy with stale data in durable mode.
BackgroundServiceHealthCheck.CheckHealthAsync only marks a service as Degraded when LastSuccessfulTicks has an entry that's too old. When no entry exists at all (first boot or never ticked), it falls into the "No tick recorded yet" branch and returns Healthy. In durable mode:
ExecuteAsyncis never started (the service is a singleton, not anIHostedService).RunAsync→CheckAndSendReminders→ returns.RecordTickis only at line 41, insideExecuteAsync's loop — it is never reached.LastSuccessfulTicksremains empty for all 9 monitored services.- After 3 minutes (the
ReminderSchedulergrace period),GET /healthstill returns200 Healthy: "All background services running"even if Hangfire is completely broken.
The same gap exists in all 10 RunAsync implementations — each delegates directly to its inner work method and skips RecordTick.
Fix: call BackgroundServiceHealthCheck.RecordTick(Name) at the end of each RunAsync, mirroring the ExecuteAsync loop. For example:
| public Task RunAsync(CancellationToken cancellationToken) => CheckAndSendReminders(cancellationToken); | |
| public async Task RunAsync(CancellationToken cancellationToken) | |
| { | |
| await CheckAndSendReminders(cancellationToken); | |
| BackgroundServiceHealthCheck.RecordTick("ReminderScheduler"); | |
| } |
The same one-liner needs adding to the other 8 services (or 9 for AccountDeletionService, which already has the right pattern in its ExecuteAsync loop and just needs it mirrored in RunAsync).
There was a problem hiding this comment.
Two well-scoped, flag-gated durability upgrades with clean separation of concerns: the Redis IDistributedCache seam swap is sound, the Hangfire integration is architecturally solid, and the test suite covers the runner dispatch and registry uniqueness. One correctness issue before merging.
BackgroundServiceHealthCheck is silently broken in durable mode. When UseDurableQueue=true, all 10 scheduler services are registered as singletons — not IHostedService — so ExecuteAsync never starts. Every RunAsync implementation delegates directly to its inner work method and skips the BackgroundServiceHealthCheck.RecordTick() call that lives only in the ExecuteAsync polling loop. The health check's LastSuccessfulTicks dictionary stays empty for all 9 monitored services forever. Because the check only marks a service Degraded when a recorded tick is too old (not when no tick exists), it returns Healthy: "All background services running" indefinitely — even if Hangfire is completely non-functional. The fix is to call RecordTick(Name) at the end of each RunAsync, mirroring what the ExecuteAsync loop already does. Inline comment on ReminderSchedulerService.cs:28 shows the pattern for all 10.
thomasluizon
left a comment
There was a problem hiding this comment.
Code Review: PR #214 (orbit-api)
Scope: PR #214 — Redis distributed cache + durable background job queue (#217 #218)
Recommendation: APPROVE
Summary
Adds (1) a Redis-backed IDistributedCache with an in-proc memory fallback, used to cache
user date preferences in UserDateService (15-min TTL), and (2) a durable execution mode
for the 10 recurring schedulers: behind BackgroundServices:UseDurableQueue they register
as Hangfire/PostgreSQL recurring jobs instead of in-process polling loops. Both features
are flag-gated and default OFF. Security review is clean on all six dimensions;
correctness, architecture, and backend hard rules all pass; build + tests green.
Findings
Critical / High / Medium
None.
Low / Info
[LOW] Durable mode reuses the app DB connection, which must hold CREATE for Hangfire's schema bootstrap
· dimension: Security / Insecure design (least-privilege, defense-in-depth)
· location: src/Orbit.Api/Extensions/ServiceCollectionExtensions.cs:482-493
· issue: When UseDurableQueue=true, Hangfire.PostgreSql auto-creates its `hangfire` schema
using ConnectionStrings:DefaultConnection (the same role EF uses), which needs CREATE/DDL.
· risk: Widens the blast radius of an existing SQLi/credential leak; no new privilege
escalation (the app already uses this connection for all EF work). Acceptable as-is given
the flag defaults OFF.
· fix (optional): bootstrap the schema once via an admin/migration role, then run the
runtime on a DML-only role; or use a dedicated least-privilege Hangfire connection string.
[INFO] Queue uses Hangfire-on-Postgres while the cache uses Redis — the right split, not an inconsistency.
· Redis is ephemeral cache (safe to lose). The durable jobs want transactional durability +
a distributed lock alongside the app's own Postgres, and Hangfire.PostgreSql delivers that
without making Redis a second critical-path datastore. The recurring scans are low-volume
(10 cron jobs), so Postgres is more than adequate. Sound decision.
[INFO] Durable mode also unlocks safe horizontal scaling.
· In-process mode (default) requires a single instance to avoid double-processing. In durable
mode, Hangfire's storage-level distributed lock ensures each occurrence runs once even
across multiple instances — so the durable path is what makes a multi-instance fleet safe.
What's good
- Clean either/or switch (
ServiceCollectionExtensions:459-462):if (useDurableQueue) AddDurableRecurringJobs() else AddInProcessSchedulers(). In durable mode each scheduler isAddSingleton<TJob>()(concrete), neverAddHostedService, so theBackgroundService.ExecuteAsyncpolling loop does not start — only Hangfire'sRunAsyncfires. No double-processing; the two modes are mutually exclusive. - Single Hangfire entry point (
ScheduledJobRunner+HangfireRecurringJobRegistrar): Hangfire persists only the job name, and the runner resolves the job per execution — so adding or renaming a job never invalidates the serialized recurring-job payload. Re-registering on every boot reconciles cron changes by stable name without creating duplicates. - Cache invalidation is correctly wired:
IUserDateService.InvalidateUserDatePreferencesis called from bothSetTimezoneCommand:32andSetWeekStartDayCommand:32, so a tz/week-start change busts the cache immediately rather than going stale for up to 15 minutes. - Fail-fast guards on both the Hangfire connection string (
:485-487) and the Redis connection string when enabled; disabled Redis falls back toAddDistributedMemoryCache(), preserving theIDistributedCacheseam. - Doc comments are
/** */-style XML-doc on exported types only; structuredLoggerMessagelogging throughout (PascalCase props, English); no narration, noany, noconsole/Console.WriteLine.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | PASS (worktree-accurate) — no Hangfire dashboard mapped anywhere; flags default OFF; no secrets logged; cached payload non-sensitive (tz prefs); job names are compile-time constants (no untrusted deserialization). One Low above. |
| contract-aligner | N/A (no DTO/shared-shape changes) |
| parity-checker / i18n-syncer | N/A (backend infra) |
Validation
| Check | Result |
|---|---|
| Build (api) | PASS (0 errors; pre-existing SQLitePCLRaw NU1903 warning only) |
| Tests | PASS — Domain 7, Application 185, Infrastructure 120; new ScheduledJobRegistryTests, ScheduledJobRunnerTests, UserDateServiceTests all green |
| Backend hard rules | PASS — no new endpoints (no [Authorize] gap); user "today" via tz conversion; structured logging; new services unit-tested |
| Contract / backward-compat | N/A (no DTO changes → no old-client risk) |
Recommendation
APPROVE. Manual rollout (operator): provision Redis → set Redis:ConnectionString +
Redis:Enabled=true; to enable durable jobs set BackgroundServices:UseDurableQueue=true
(Hangfire auto-creates its schema on first boot). Deploy the API first, then flip the
flags. Closes #217 #218.
In durable mode (UseDurableQueue=true) schedulers run as Hangfire jobs via RunAsync, not the in-process ExecuteAsync loop, so BackgroundServiceHealthCheck never recorded a tick and the check reported Healthy forever even if Hangfire was dead (a never-ticked service is not flagged stale). Each RunAsync now records its tick with the same PascalCase key its ExecuteAsync loop uses (matching ExpectedIntervals); the bot-suggested Name is the kebab job id and would not match the health-check keys. Addresses the PR-review bot's change request. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| public async Task RunAsync(CancellationToken cancellationToken) | ||
| { | ||
| await ProcessTick(cancellationToken); | ||
| BackgroundServiceHealthCheck.RecordTick("CalendarAutoSync"); |
There was a problem hiding this comment.
The tick is recorded here, but "CalendarAutoSync" is absent from ExpectedIntervals in BackgroundServiceHealthCheck. CheckHealthAsync only evaluates keys in that dictionary — ticks for unknown keys are stored in LastSuccessfulTicks but never compared against a threshold. All 9 other migrated schedulers have an entry; this one doesn't.
Fix: add ["CalendarAutoSync"] = TimeSpan.FromMinutes(45) (3× the 15-min cron interval) to ExpectedIntervals in BackgroundServiceHealthCheck.cs.
| public class UserDateService( | ||
| IGenericRepository<User> userRepository, | ||
| IMemoryCache cache) : IUserDateService | ||
| IDistributedCache cache) : IUserDateService |
There was a problem hiding this comment.
IDistributedCache.Remove(string key) (called in InvalidateUserDatePreferences) is a sync-over-async shim — it's defined in DistributedCacheExtensions as cache.RemoveAsync(key).GetAwaiter().GetResult(). With Redis enabled, this blocks a thread pool thread on a network round-trip for every timezone/week-start update. The old IMemoryCache.Remove was genuinely synchronous; IDistributedCache.Remove is not.
The fix requires changing the interface signature and four files:
IUserDateService→Task InvalidateUserDatePreferencesAsync(Guid userId)UserDateService→public async Task InvalidateUserDatePreferencesAsync(Guid userId) => await cache.RemoveAsync(CacheKey(userId));SetTimezoneCommand.csline 32 →await userDateService.InvalidateUserDatePreferencesAsync(...)SetWeekStartDayCommand.csline 32 → same
There was a problem hiding this comment.
Two correctness issues before merging.
1. CalendarAutoSync missing from health check ExpectedIntervals — The fix commit (10b39f1) correctly added RecordTick("CalendarAutoSync") to CalendarAutoSyncService.RunAsync, but "CalendarAutoSync" was never added to ExpectedIntervals in BackgroundServiceHealthCheck. CheckHealthAsync only evaluates keys that exist in that dictionary, so ticks are written but never checked. CalendarAutoSyncService can stall or die in durable mode with the health endpoint permanently returning Healthy. All 9 other migrated schedulers have an entry; this one was missed. Fix: add ["CalendarAutoSync"] = TimeSpan.FromMinutes(45) to ExpectedIntervals.
2. InvalidateUserDatePreferences is sync-over-async against IDistributedCache — IDistributedCache.Remove(string key) (used on line 54 of UserDateService.cs) is defined in DistributedCacheExtensions as RemoveAsync(...).GetAwaiter().GetResult(). With Redis enabled, every timezone or week-start update blocks a thread pool thread on a Redis network round-trip. The previous IMemoryCache.Remove was genuinely synchronous; this replacement is not. Fix requires changing IUserDateService.InvalidateUserDatePreferences to return Task, updating the UserDateService implementation to call await cache.RemoveAsync(...), and awaiting the call in SetTimezoneCommand and SetWeekStartDayCommand.
- Add CalendarAutoSync to BackgroundServiceHealthCheck.ExpectedIntervals: it records a tick in both modes but was never evaluated, so a stall went unnoticed (dead tick). - IUserDateService.InvalidateUserDatePreferences was sync-over-async on IDistributedCache (Redis): IDistributedCache.Remove blocks a thread-pool thread on a network round-trip. Make it InvalidateUserDatePreferencesAsync returning Task, call RemoveAsync, and await it in SetTimezone/SetWeekStartDay. Addresses the PR-review bot's two change requests. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All three prior change requests are resolved. The CalendarAutoSync entry is now wired into ExpectedIntervals (45-minute threshold), InvalidateUserDatePreferences is async end-to-end (RemoveAsync + cancellation token threaded through both callers), and durable-mode health ticks are recorded in every RunAsync. The flag-gated Redis and Hangfire wiring is sound, the either/or DI switch ensures the two modes never run simultaneously, and the test suite covers runner dispatch, registry uniqueness, and cross-instance cache consistency. Good to merge.
|


Summary
Two flag-gated backend-durability changes from the Phase 2 Hardening & Tooling milestone. Both are off by default, keep the existing path fully intact, and are append-only (no behavior change until an operator flips the flag).
Tracking issues live in the consumer repo:
#217 — Redis distributed cache for user-date preferences
UserDateServicecached each user's timezone + week-start in a per-instanceIMemoryCache, so with more than one API instance a preference change could be served stale by the instances that didn't handle the write (up to the 15-minute TTL).Change:
UserDateServicenow depends on the frameworkIDistributedCacheseam instead ofIMemoryCache. The backing store is chosen by a rollout flag:Redis:Enabled = false(default) →AddDistributedMemoryCache()registers an in-processIDistributedCache; behavior is identical to before.Redis:Enabled = true→AddStackExchangeRedisCache(...)backs the same seam with Redis, so all instances read one shared value.Preferences serialize as JSON with the existing 15-minute TTL, and a preference change still calls
InvalidateUserDatePreferences(aIDistributedCache.Remove). This moves only the user-prefs cache — the unrelated AI-contentIMemoryCachecaches (daily summary, retrospective, etc.) are untouched, andAddMemoryCache()stays registered for them.No frontend changes.
#218 — Durable Hangfire job queue for recurring schedulers
The recurring schedulers were hand-rolled
BackgroundServicepolling loops (while (!stopping) { scan(); await Task.Delay(interval); }). On a multi-instance deployment every instance runs every scan concurrently; a restart abandons the current tick; a transient failure just waits a full interval before the next attempt.Change: a flag-gated durable path on Hangfire + PostgreSQL.
BackgroundServices:UseDurableQueue = false(default) → each scheduler runs as its existing in-processBackgroundService, unchanged.BackgroundServices:UseDurableQueue = true→ the 10 recurring scans are registered as Hangfire recurring jobs (Postgres storage, dedicated Hangfire schema). Occurrences persist across restarts, Hangfire's distributed lock ensures a single instance runs each occurrence, and failed runs retry with exponential backoff. In this mode the in-process loops are not registered, so the two paths never run simultaneously.Queue backing-store decision: PostgreSQL (not Redis)
Chosen
Hangfire.PostgreSqlover a Redis-backed queue (and over "reuse the #217 Redis"):Hangfire.PostgreSqlis the mature, first-class storage provider; Redis-as-Hangfire-storage is a less standard community path.SentReminders,SentSlipAlerts, etc.); keeping the scheduler/lock state in the same database avoids split-brain between the lock store and the dedup store.Redis (#217) is for the read-through preferences cache; Postgres (#218) is for the durable job/lock store. They are deliberately separate concerns.
Design
IScheduledJob(Name,CronExpression,RunAsync) — each recurring scheduler implements it, delegatingRunAsyncto the scan its polling loop already runs. No behavior change when the flag is off.ScheduledJobRunner— the single Hangfire entry point. Hangfire persists only the job name; the runner resolves the matchingIScheduledJoband runs it, so renaming/adding jobs never changes the serialized recurring-job payload.HangfireRecurringJobRegistrar(IHostedService) — reconciles every job's schedule on startup (AddOrUpdatekeyed by name, idempotent).DataEncryptionMigrationServicestays a hosted service in both modes (it must run once at startup; recurring scheduling does not apply).Schedulers migrated (10) + cron (mirrors current default interval)
Name)reminder-scheduler* * * * *goal-deadline-notification*/30 * * * *slip-alert-scheduler*/5 * * * *habit-due-date-advancement*/30 * * * *streak-goal-sync0 * * * *streak-freeze-auto-activation0 * * * *calendar-auto-sync*/15 * * * *account-deletion0 3 * * *sync-cleanup30 3 * * *play-notification-cleanup0 4 * * *Not migrated (intentional):
DataEncryptionMigrationService— one-shot startup migration, stays hosted in both modes.Remaining as a follow-up (out of this PR's AC)
awaitIEmailServiceinline and so block the HTTP response on the email send:SendCodeCommand,VerifyCodeCommand,RequestAccountDeletionCommand,GoogleAuthCommand,SendSupportCommand. (For comparison,CheckReferralCompletionCommandalready fires its push fire-and-forget viaTask.Run, andGamificationServicepush is only reached from background/async contexts — neither blocks a request.) The scheduled push/email paths already run off-request via the schedulers; offloading these request-path email sends is a separate, broader refactor (theApplicationlayer has no Hangfire reference by design, so it needs a dispatcher seam + per-message job types across the auth flows) and is deliberately left for a focused follow-up rather than widening this PR beyond the feat(api): object-storage sign endpoint (Supabase Storage) (#216) #218 acceptance criteria.Operator setup required before enabling either flag
All keys are read from configuration; on Render set them as environment variables (
__is the nesting separator). Deploy this build first; flip flags afterward.#217 Redis (
Redissection)Redis__Enabledtruefalseto keep in-memoryRedis__ConnectionString<render-key-value-internal-url>Redis__InstanceNameorbit:Redis__ConnectionString, then setRedis__Enabled=true.Enabled=truewith an empty connection string.#218 Durable queue (
BackgroundServicessection)BackgroundServices__UseDurableQueuetruefalsefor in-process loopsConnectionStrings__DefaultConnection(the existing Supabase Postgres). On first boot with the flag on, Hangfire creates its ownhangfireschema (requires the DB role used by the app to have CREATE on the database — the Supabase app role already does).UseHangfireDashboard), so no new auth surface.Validation
dotnet build Orbit.slnx— succeeds, 0 errors (2 pre-existing warnings unrelated to this change: an NU1903 SQLite advisory in the test project and a CS8602 in an untouched test).dotnet test Orbit.slnx— 3526 passed, 0 failed (Domain 386 / Application 2001 / Infrastructure 1139).UserDateServiceTests— distributed-cache read/cache-on-first-call, invalidation forces a reload, and a second instance sharing one cache reads the value without hitting its own repository (the cross-instance consistency AC).ScheduledJobRunnerTests— dispatch routes to the named job, throws on an unknown name, forwards the cancellation token.ScheduledJobRegistryTests— all 10 schedulers exposeIScheduledJob, job names are unique (a duplicate would silently overwrite/lose a job), andRunAsyncexecutes the underlying scan without firing duplicate side effects.NuGet added
Microsoft.Extensions.Caching.StackExchangeRedis10.0.2(Api) — matches the pinned framework line.Hangfire.Core1.8.23(Infrastructure),Hangfire.AspNetCore1.8.23+Hangfire.PostgreSql1.21.1(Api).🤖 Generated with Claude Code