[Reliability][Go-live] Eliminate availability bombs — Redis scans/flush, MongoClient-per-message, GPU ledger, poison requeue #411

New Issue

2026-07-05T20:24:39Z

spikerj commented

2026-07-05 20:24:39 +00:00

Problem — availability bombs that degrade production without any attacker:

The L2 cache runs a full-cluster SCAN * on every cache miss; InvalidateAll serially deletes every key across the shared cluster.
A game-event consumer builds a new MongoClient per message → connection/socket exhaustion.
GpuCoordinator keeps its VRAM ledger only in memory → a restart resets it to zero while workers still hold GPUs → over-allocation / OOM.
Poison-message infinite-requeue loops pin CPU on a single bad message.

Evidence:

SpikerSoft.Business/Services/RedisL2CacheService.cs:123-140,277-288; Behaviors/CacheInvalidationBehavior.cs:49-54
SpikerSoft.EventHandlers.GameEvents/Services/GamePersistenceService.cs:56 (AddScoped, per-message)
SpikerSoft.EventHandlers.GpuCoordinator/Services/LeaseManagerService.cs:161-168
Requeue loops: KeycloakEvents/Services/KeycloakEventHostedService.cs:188-192, GpuCoordinator/Services/LeaseManagerService.cs:288-292

Fix: Delete the debug SCAN blocks / scope invalidation to domain prefixes; register a singleton IMongoClient; persist the GPU ledger (Redis is in-stack); nack poison messages requeue:false to a DLQ with bounded backoff.

Acceptance criteria: No SCAN */KEYS on hot paths; one shared Mongo client; GPU ledger survives a restart; poison messages dead-letter instead of looping.

Effort: S–M · Related: #371 (Redis cluster persistence incident).

**Problem — availability bombs that degrade production without any attacker:** 1. The L2 cache runs a full-cluster `SCAN *` on **every cache miss**; `InvalidateAll` serially deletes every key across the shared cluster. 2. A game-event consumer builds a new `MongoClient` **per message** → connection/socket exhaustion. 3. GpuCoordinator keeps its VRAM ledger only in memory → a restart resets it to zero while workers still hold GPUs → over-allocation / OOM. 4. Poison-message infinite-requeue loops pin CPU on a single bad message. **Evidence:** - `SpikerSoft.Business/Services/RedisL2CacheService.cs:123-140,277-288`; `Behaviors/CacheInvalidationBehavior.cs:49-54` - `SpikerSoft.EventHandlers.GameEvents/Services/GamePersistenceService.cs:56` (`AddScoped`, per-message) - `SpikerSoft.EventHandlers.GpuCoordinator/Services/LeaseManagerService.cs:161-168` - Requeue loops: `KeycloakEvents/Services/KeycloakEventHostedService.cs:188-192`, `GpuCoordinator/Services/LeaseManagerService.cs:288-292` **Fix:** Delete the debug `SCAN` blocks / scope invalidation to domain prefixes; register a singleton `IMongoClient`; persist the GPU ledger (Redis is in-stack); nack poison messages `requeue:false` to a DLQ with bounded backoff. **Acceptance criteria:** No `SCAN *`/`KEYS` on hot paths; one shared Mongo client; GPU ledger survives a restart; poison messages dead-letter instead of looping. **Effort:** S–M · Related: #371 (Redis cluster persistence incident).

spikerj added the agentic label 2026-07-05 20:24:39 +00:00

spikerj commented

2026-07-06 03:02:31 +00:00

Triage — "poison requeue" sub-item (reframed after code check)

Update: the MongoClient-per-message bomb is fixed in backend PR #111 (injects the singleton IMongoClient; GamePersistenceService was AddScoped + scoped-per-message).

On poison requeue — it's not a missing-mechanism bug. The messaging layer already has a retry-cap + dead-letter pattern: RabbitMQRetryHelper.GetRetryCount(ea) + ClassifyError(...) gated on _dlqConfig. Verified directly in UploadCoordinator/FileMovedConsumer.cs:

if (_dlqConfig is null || !_dlqConfig.Enabled) { /* requeue: true — fallback only */ }
var retryCount = RabbitMQRetryHelper.GetRetryCount(ea);
var errorType  = RabbitMQRetryHelper.ClassifyError(exception);

So the BasicNack(..., requeue: true) calls a grep flags are mostly the DLQ-disabled fallback branch, not unconditional infinite requeue.

Therefore the real sub-item is a coverage/config audit, not a rewrite:

Confirm DeadLetterQueue.Enabled = true in every consumer's deployed appsettings (a single disabled config re-opens the infinite-requeue path).
Per-consumer audit that each failure path routes through RabbitMQRetryHelper — the RPC/hosted-service consumers (Keycloak, GpuCoordinator lease, CodeExecution, Embeddings, QuizGeneration) are the ones to eyeball, since some legitimately requeue: true for transient backpressure and may or may not cap.

(I couldn't produce a trustworthy per-consumer list by grep here — the repo path contains a space that broke my shell word-splitting — so flagging the method rather than a shaky inventory.) The other #411 bombs (Redis scans/flush, GPU ledger in-memory state) remain separate.

### Triage — "poison requeue" sub-item (reframed after code check) Update: the **MongoClient-per-message** bomb is fixed in backend PR #111 (injects the singleton `IMongoClient`; `GamePersistenceService` was `AddScoped` + scoped-per-message). On **poison requeue** — it's *not* a missing-mechanism bug. The messaging layer already has a retry-cap + dead-letter pattern: `RabbitMQRetryHelper.GetRetryCount(ea)` + `ClassifyError(...)` gated on `_dlqConfig`. Verified directly in `UploadCoordinator/FileMovedConsumer.cs`: ```csharp if (_dlqConfig is null || !_dlqConfig.Enabled) { /* requeue: true — fallback only */ } var retryCount = RabbitMQRetryHelper.GetRetryCount(ea); var errorType = RabbitMQRetryHelper.ClassifyError(exception); ``` So the `BasicNack(..., requeue: true)` calls a grep flags are mostly the **DLQ-disabled fallback branch**, not unconditional infinite requeue. **Therefore the real sub-item is a coverage/config audit, not a rewrite:** 1. Confirm `DeadLetterQueue.Enabled = true` in every consumer's deployed `appsettings` (a single disabled config re-opens the infinite-requeue path). 2. Per-consumer audit that each failure path routes through `RabbitMQRetryHelper` — the RPC/hosted-service consumers (Keycloak, GpuCoordinator lease, CodeExecution, Embeddings, QuizGeneration) are the ones to eyeball, since some legitimately `requeue: true` for transient backpressure and may or may not cap. (I couldn't produce a trustworthy per-consumer list by grep here — the repo path contains a space that broke my shell word-splitting — so flagging the method rather than a shaky inventory.) The other #411 bombs (Redis scans/flush, GPU ledger in-memory state) remain separate.

spikerj referenced this issue

2026-07-06 03:05:46 +00:00

[Reliability] MongoDB_HealthCheck constructs a new MongoClient on every invocation (connection-pool leak) #433

spikerj commented

2026-07-06 03:08:41 +00:00

Progress: the MongoClient-per-message bomb is now merged (PR #111 → master, GamePersistenceService reuses the singleton IMongoClient). A related per-invocation MongoClient leak in MongoDB_HealthCheck was split into #433 (PR #112, in review).

Keeping this epic open for the remaining availability bombs: Redis scans/FLUSH, the in-memory GPU-lease ledger, and the poison-requeue coverage audit (see prior comment — the DLQ/RabbitMQRetryHelper mechanism exists; needs a config/coverage pass rather than a rewrite).

Progress: the **MongoClient-per-message** bomb is now **merged** (PR #111 → `master`, `GamePersistenceService` reuses the singleton `IMongoClient`). A related per-invocation `MongoClient` leak in `MongoDB_HealthCheck` was split into #433 (PR #112, in review). Keeping this epic **open** for the remaining availability bombs: Redis scans/`FLUSH`, the in-memory GPU-lease ledger, and the poison-requeue coverage audit (see prior comment — the DLQ/`RabbitMQRetryHelper` mechanism exists; needs a config/coverage pass rather than a rewrite).

spikerj referenced this issue

2026-07-06 03:11:07 +00:00

[Reliability] Scheduler TaskRunners create a new HttpClient per execution (socket exhaustion — use IHttpClientFactory) #434

spikerj referenced this issue

2026-07-06 03:18:28 +00:00

[Reliability] KeycloakAdminService.GetRoleMembersAsync news a per-call HttpClient (leak + inconsistent with the rest of the class) #435

spikerj commented

2026-07-06 05:20:06 +00:00

Item 4 (poison requeue) — audited both cited sites; findings differ from the ticket

KeycloakEventHostedService (cited 188-192) is already protected — not a bug. Its HandleMessageFailureAsync runs the full machinery: RabbitMQRetryHelper retry-cap, _poisonDetector.RecordFailure, _dlqMetrics, and DLQ routing. The BasicNackAsync(requeue:true) at 188 is only the DLQ-disabled fallback — and DLQ is Enabled: true, MaxRetries: 3 in both appsettings.Production.json and appsettings.Development.json. So the infinite-requeue path is never reached in any configured environment. No change needed here.

LeaseManagerService (GpuCoordinator) is the real gap. It has no DLQ/DLX/retry infrastructure at all, and three unconditional infinite-requeue poison loops:

LeaseManagerService.cs:291 — OnLeaseRequestReceived
LeaseManagerService.cs:355 — OnTaskCompleteReceived
LeaseManagerService.cs:381 — OnReleaseReceived

Each is catch (Exception) { … BasicNackAsync(deliveryTag, multiple:false, requeue:true); } — a message that always throws pins the consumer.

Why I'm not blind-shipping this: the safe fix is not just flipping requeue:true → false. Without a dead-letter exchange, requeue:false drops the message — and dropping a release message would leak a GPU lease (the GPU never gets freed), which is worse than the loop. The correct fix mirrors the Keycloak/EventHandlerHostBuilder pattern: declare the 3 GPU queues with an x-dead-letter-exchange, add RabbitMQRetryHelper retry-cap, and dead-letter after N attempts. That touches queue declaration (and the GpuCoordinator is a bespoke host, not on the shared DLQ infra) and needs broker verification that poison messages land in the DLQ rather than vanish.

Recommendation: scope item 4 to just LeaseManagerService — port it onto the shared DLQ/retry infra (the same DeadLetterQueue config block + RabbitMQRetryHelper Keycloak already uses). I can implement it, but it wants a broker-integration check before merge, so flagging rather than auto-shipping. Items 1 (#122) and 2 (#111) are done; item 3 (GPU ledger persistence) is the other remaining piece.

### Item 4 (poison requeue) — audited both cited sites; findings differ from the ticket **`KeycloakEventHostedService` (cited 188-192) is already protected — not a bug.** Its `HandleMessageFailureAsync` runs the full machinery: `RabbitMQRetryHelper` retry-cap, `_poisonDetector.RecordFailure`, `_dlqMetrics`, and DLQ routing. The `BasicNackAsync(requeue:true)` at 188 is *only* the `DLQ-disabled` fallback — and DLQ is `Enabled: true, MaxRetries: 3` in **both** `appsettings.Production.json` and `appsettings.Development.json`. So the infinite-requeue path is never reached in any configured environment. No change needed here. **`LeaseManagerService` (GpuCoordinator) is the real gap.** It has **no** DLQ/DLX/retry infrastructure at all, and **three** unconditional infinite-requeue poison loops: - `LeaseManagerService.cs:291` — `OnLeaseRequestReceived` - `LeaseManagerService.cs:355` — `OnTaskCompleteReceived` - `LeaseManagerService.cs:381` — `OnReleaseReceived` Each is `catch (Exception) { … BasicNackAsync(deliveryTag, multiple:false, requeue:true); }` — a message that always throws pins the consumer. **Why I'm not blind-shipping this:** the safe fix is *not* just flipping `requeue:true → false`. Without a dead-letter exchange, `requeue:false` **drops** the message — and dropping a `release` message would **leak a GPU lease** (the GPU never gets freed), which is worse than the loop. The correct fix mirrors the Keycloak/`EventHandlerHostBuilder` pattern: declare the 3 GPU queues with an `x-dead-letter-exchange`, add `RabbitMQRetryHelper` retry-cap, and dead-letter after N attempts. That touches queue declaration (and the GpuCoordinator is a bespoke host, not on the shared DLQ infra) and needs broker verification that poison messages land in the DLQ rather than vanish. **Recommendation:** scope item 4 to *just* `LeaseManagerService` — port it onto the shared DLQ/retry infra (the same `DeadLetterQueue` config block + `RabbitMQRetryHelper` Keycloak already uses). I can implement it, but it wants a broker-integration check before merge, so flagging rather than auto-shipping. Items 1 (#122) and 2 (#111) are done; item 3 (GPU ledger persistence) is the other remaining piece.

spikerj commented

2026-07-06 06:06:42 +00:00

PR #122 merged to master (9d74b44) — item 1 (full-cluster SCAN * on every cache miss) eliminated.

Epic status: item 1 ✅ (#122), item 2 ✅ (#111, MongoClient-per-message). Remaining: item 3 (GPU ledger persistence) and item 4 (LeaseManagerService DLQ — see the audit above; needs DLX + broker verification). Keeping open.

**PR #122 merged to master** (`9d74b44`) — item 1 (full-cluster `SCAN *` on every cache miss) eliminated. Epic status: **item 1 ✅ (#122)**, **item 2 ✅ (#111, MongoClient-per-message)**. Remaining: item 3 (GPU ledger persistence) and item 4 (`LeaseManagerService` DLQ — see the audit above; needs DLX + broker verification). Keeping open.

spikerj referenced this issue

2026-07-06 16:05:56 +00:00

[Correctness][Go-live] Repair or gate non-functional features (Trellis 3D, decompiler, phone lookup) #412

spikerj commented

2026-07-22 02:58:15 +00:00

Board-sweep status (2026-07-22): items 1+2 merged (SCAN* removed #122; singleton MongoClient #111). REMAINING: item 3 GPU-ledger persistence + item 4 poison-message DLQ in LeaseManagerService.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#411