fix(gpu): harden multi-user GPU contention — gate /health, align Ollama, jittered desk retry (ADR-0040)

docker-compose.yml

@@ -20,6 +20,17 @@ services:
       # set anywhere). 4096 fits Phase 0 structuring prompts; revisit for the
       # Phase 4 synthesis window. Larger costs VRAM per loaded model.
       OLLAMA_CONTEXT_LENGTH: ${OLLAMA_CONTEXT_LENGTH:-4096}
+      # Concurrency MUST agree with the app-side LlmGate (Ingest:LlmMaxConcurrency
+      # = 2, shed-not-queue), or the two layers fight: the gate fast-sheds at 2
+      # while Ollama silently deep-queues under it (default OLLAMA_MAX_QUEUE=512),
+      # so a call the gate admits can still stall for minutes in a hidden backlog
+      # bounded only by Ingest:LlmCallTimeoutMs. NUM_PARALLEL=2 lets both admitted
+      # slots actually run in parallel instead of being serialized; a SHALLOW
+      # queue makes Ollama shed too rather than hide a deep one (ADR-0040). Safe
+      # on VRAM because this container runs only while the shared RAG is down, so
+      # the whole GPU is ours. Tunable via env if a future GPU is tighter.
+      OLLAMA_NUM_PARALLEL: ${OLLAMA_NUM_PARALLEL:-2}
+      OLLAMA_MAX_QUEUE: ${OLLAMA_MAX_QUEUE:-8}
     ports:
       - "11434:11434"
     healthcheck:

docs/decisions/0040-gpu-contention-hardening.md

@@ -0,0 +1,88 @@
+# ADR-0040 — GPU contention under many simultaneous users: shed, don't queue (and make the layers agree)
+
+- **Status:** Accepted (2026-07-14)
+- **Deciders:** Mikko
+
+## Context
+
+The app runs one 8B model (Poro-2-8B, Q4) on a single GPU that is time-shared
+with a sibling project's RAG. A demo is shown to a room, so "many people use it
+at once" is the expected load, not an edge case. The question: what happens when
+concurrent users all need the GPU, and do we queue?
+
+The existing design already sheds rather than queues (`LlmGate`, a 2-slot
+semaphore: a caller waits at most `LlmAcquireTimeoutMs = 500 ms` for a slot,
+then gets `LlmBusyException` → HTTP 503). A concurrency audit confirmed the
+shed is the right call for an interactive desk, but found three sharp edges:
+
+1. **`/health` bypassed the gate.** It fired a real 1-token completion on every
+   probe with no slot acquisition, so health checks added uncontrolled
+   concurrent GPU calls on top of the 2 slots and could steal a slot from a
+   live request under load.
+2. **Ollama's own concurrency was unconfigured.** Beneath the app's fast-shed
+   gate, `ollama serve` ran at defaults — `OLLAMA_MAX_QUEUE = 512`. A call the
+   gate admits could then stall for minutes in a *hidden* deep queue bounded
+   only by `LlmCallTimeoutMs = 120 s`. The two layers disagreed: the app
+   fast-sheds, Ollama deep-queues.
+3. **A shed on the desk had no client resilience.** A single transient 503
+   dropped the clerk straight to the busy message, and a roomful of manual
+   re-clicks could re-collide in lockstep and also synchronize into the shared
+   rate-limit bucket (one Azure egress IP → one 240/60 s bucket, ADR-0025).
+
+### Why not just use Ollama's queue?
+
+Because we do not want a deep queue at all. On one 8B GPU a 512-deep queue means
+an interactive "Tulkitse" click could wait minutes; a fast "busy, try again"
+beats a spinner that resolves in three minutes. And the app-level gate expresses
+policy Ollama cannot: cap *this* app at 2 concurrent so it stays a good neighbour
+to the sibling RAG, distinguish shed-vs-degrade per endpoint (ingest sheds and
+stores nothing so the client retries; the report swallows the shed and renders
+its deterministic layer), and enforce the per-call timeout. Ollama's queue is
+tuned for batch throughput, not latency-bounded interactivity. The fix is not to
+adopt Ollama's queue but to make Ollama *agree* with the gate.
+
+## Decision
+
+1. **Gate `/health`.** The probe acquires a slot **non-blocking**
+   (`LlmGate.TryRunAsync`, `WaitAsync(0)`): if a slot is free it runs the
+   1-token completion; if the gate is saturated it returns `status: "ok"`
+   without a completion, because a full gate already proves the model is loaded
+   and generating. Health can never add GPU load beyond the 2-slot bound nor
+   steal a slot from a live request. A genuine failure (unreachable, or slower
+   than `HealthTimeoutSeconds = 10 s`) still returns `503 llm_unavailable`.
+2. **Make Ollama agree with the gate** (`docker-compose.yml`):
+   `OLLAMA_NUM_PARALLEL = 2` (both slots the gate admits actually run in
+   parallel instead of being serialized) and `OLLAMA_MAX_QUEUE = 8` (shallow, so
+   Ollama sheds too rather than hiding a 512-deep backlog under the app's
+   fast-shed design). Both env-overridable; `NUM_PARALLEL = 2` is safe on VRAM
+   because this container runs only while the shared RAG is down, so the whole
+   GPU is ours.
+3. **Client resilience on the desk** (`desk.html`): a 503 on interpret retries a
+   few times with **jittered** backoff (`300 + attempt·400 + random·500 ms`, up
+   to 2 retries) so transient sheds self-heal and a roomful of retries
+   desynchronize instead of re-colliding; then it falls to the manual busy
+   message. A 429 (shared rate bucket, not the GPU) is never retried — it shows
+   a distinct "too many requests, wait" message.
+
+## Consequences
+
+- The two concurrency layers now express the same policy (2 parallel, shallow
+  queue, shed fast) instead of fighting; there is no hidden deep queue under the
+  gate.
+- Health is honest under load: busy → healthy without adding load; idle → real
+  probe; dead/slow → `503 llm_unavailable`. `/health` consumers
+  (`feedctl` board, the public `/api/health` proxy probe) read `status == "ok"`
+  and need no change.
+- A transient GPU shed on the desk mostly self-heals silently; a sustained one
+  still shows the busy message quickly. Auto-retry is **bounded** (≤2, jittered)
+  so it smooths sheds without becoming a retry storm.
+- **Accepted, not fixed:** `POST /live/restructure` still holds a slot per item
+  in a loop with no priority over live visitors. It is operator-only
+  (loopback-exempt, absent from the public proxy allowlist), so only the
+  operator can trigger it and they know a batch is running; a per-caller slot
+  reservation would be a larger change than the risk warrants.
+- **Accepted, not fixed:** the browser-path rate-limit bucket is shared across
+  all public visitors (Azure egress IP). At demo scale (240/60 s ≈ 4 req/s
+  sustained) this is ample; the client-side 429 message + jittered 503 retry are
+  the mitigation. A per-visitor bucket would require the proxy to forward a
+  stable per-client key, which the SWA managed function does not.

docs/decisions/README.md

@@ -50,3 +50,4 @@ these ADRs relocate that reasoning into durable records.
 | [0037](0037-category-keywords-service-premises.md) | Extend the category-keyword override to service/premises (kassa_palvelu, tilat_siisteys) declared last, so products always win and service is the fallback | Accepted |
 | [0038](0038-unrated-no-trend-no-model-narrative.md) | Unrated content carries no trend either, and demoted categories get a deterministic count-only moderation theme (no model narrative) | Accepted |
 | [0039](0039-graded-vulgarity-density-asiaton.md) | Graded vulgarity: a density-gated deterministic scorer forces dense, non-substantive profanity to `asiaton` (a lone swear in real feedback stays rated) | Accepted |
+| [0040](0040-gpu-contention-hardening.md) | GPU contention: shed-not-queue is kept; gate the `/health` probe, make Ollama's parallelism/queue agree with the 2-slot gate, jittered client retry on 503 | Accepted |

docs/operations.md

@@ -89,7 +89,7 @@ Ported rather than reinvented; each was measured there.
   |---|---|
   | Input length cap | 800 chars |
   | Request-body cap | 16 KB |
-  | LLM concurrency | 2, with 0.5 s acquire-then-**shed** (never queue behind a busy GPU) |
+  | LLM concurrency | 2, with 0.5 s acquire-then-**shed** (never queue behind a busy GPU). Ollama is configured to match — `OLLAMA_NUM_PARALLEL=2`, shallow `OLLAMA_MAX_QUEUE=8` — so no hidden deep queue sits under the gate, and `/health` probes take a slot only if one is free ([ADR-0040](decisions/0040-gpu-contention-hardening.md)) |
   | Per-IP rate limit | 30 requests / 60 s |
   | Output-token cap | `num_predict` / `MaxOutputTokens` |
 

src/FeedbackIntelligence.Api/LlmGate.cs

@@ -26,23 +26,50 @@ public async Task<T> RunAsync<T>(Func<CancellationToken, Task<T>> work, Cancella
             throw new LlmBusyException();
         try
         {
-            // Bound the generation with a server-side deadline so a HUNG model call
-            // cannot hold this slot (and the request thread) forever. The linked
-            // source cancels on EITHER the caller's request-abort OR the timeout;
-            // on timeout the work throws OperationCanceledException carrying the
-            // timeout token (ct is NOT cancelled), so callers' request-cancellation
-            // guards (`when (ct.IsCancellationRequested)`) correctly treat it as an
-            // LLM failure (structure_failed / deterministic fallback), not a client
-            // disconnect. The finally releases the slot either way.
-            using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct);
-            timeout.CancelAfter(options.Value.LlmCallTimeoutMs);
-            return await work(timeout.Token);
+            return await RunHeldAsync(work, ct);
         }
         finally
         {
             _slots.Release();
         }
     }
 
+    /// <summary>Runs <paramref name="work"/> ONLY if a slot is immediately free
+    /// (non-blocking acquire, no wait). Returns false without running when the
+    /// gate is saturated — the caller decides what a busy gate means. Used by
+    /// /health, which must never add GPU load beyond the gate nor steal a slot
+    /// from a live request: a full gate already proves the model is loaded and
+    /// generating, so "busy" is itself a healthy answer. Exceptions from the
+    /// work (e.g. the health timeout) propagate; the slot is always released.</summary>
+    public async Task<bool> TryRunAsync(Func<CancellationToken, Task> work, CancellationToken ct)
+    {
+        if (!await _slots.WaitAsync(0, ct))
+            return false;
+        try
+        {
+            await RunHeldAsync<object?>(async innerCt => { await work(innerCt); return null; }, ct);
+            return true;
+        }
+        finally
+        {
+            _slots.Release();
+        }
+    }
+
+    // Bound the generation with a server-side deadline so a HUNG model call
+    // cannot hold this slot (and the request thread) forever. The linked source
+    // cancels on EITHER the caller's request-abort OR the timeout; on timeout
+    // the work throws OperationCanceledException carrying the timeout token (ct
+    // is NOT cancelled), so callers' request-cancellation guards
+    // (`when (ct.IsCancellationRequested)`) correctly treat it as an LLM failure
+    // (structure_failed / deterministic fallback), not a client disconnect.
+    // Assumes a slot is already held; the caller's finally releases it.
+    private async Task<T> RunHeldAsync<T>(Func<CancellationToken, Task<T>> work, CancellationToken ct)
+    {
+        using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct);
+        timeout.CancelAfter(options.Value.LlmCallTimeoutMs);
+        return await work(timeout.Token);
+    }
+
     public void Dispose() => _slots.Dispose();
 }

src/FeedbackIntelligence.Api/Program.cs

@@ -414,15 +414,26 @@ await reports.ReadLatestSnapshotHtmlAsync(ct) is { } html
         : Results.NotFound());
 
 // Health = a 1-token REAL completion (RAG-measured pattern): "server up" does
-// not mean "model loaded and generating".
-app.MapGet("/health", async (IServiceProvider services, IOptions<IngestOptions> options, CancellationToken ct) =>
+// not mean "model loaded and generating". GATED (ADR-0040): the probe takes a
+// slot ONLY if one is immediately free, so it can never add GPU load beyond the
+// 2-slot LlmGate nor steal a slot from a live request under load. A saturated
+// gate already proves the model is loaded and generating, so TryRunAsync
+// returning false ("busy") is itself a healthy answer — no probe needed.
+app.MapGet("/health", async (
+    IServiceProvider services, LlmGate gate, IOptions<IngestOptions> options, CancellationToken ct) =>
 {
     var client = services.GetRequiredKeyedService<IChatClient>(LlmServiceCollectionExtensions.StructuringKey);
     try
     {
-        using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct);
-        timeout.CancelAfter(TimeSpan.FromSeconds(options.Value.HealthTimeoutSeconds));
-        _ = await client.GetResponseAsync("ping", new ChatOptions { MaxOutputTokens = 1 }, timeout.Token);
+        // false = gate full = model busy = up (no completion run). true = ran
+        // the 1-token probe successfully. A throw = the model is unreachable or
+        // too slow to answer within HealthTimeoutSeconds → caught below.
+        await gate.TryRunAsync(async slotCt =>
+        {
+            using var timeout = CancellationTokenSource.CreateLinkedTokenSource(slotCt);
+            timeout.CancelAfter(TimeSpan.FromSeconds(options.Value.HealthTimeoutSeconds));
+            _ = await client.GetResponseAsync("ping", new ChatOptions { MaxOutputTokens = 1 }, timeout.Token);
+        }, ct);
         return Results.Ok(new { status = "ok" });
     }
     catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested)

src/FeedbackIntelligence.Api/wwwroot/desk.html

@@ -126,7 +126,7 @@ <h1 id="h1"></h1>
     interpretHeading: "Tulkinta — tarkista ja korjaa tarvittaessa",
     theme: "Aihe", severity: "Vakavuus", type: "Tyyppi", cancel: "Peruuta", save: "Tallenna",
     writeFirst: "Kirjoita palaute ensin.", interpreting: "Tulkitaan…",
-    busy: "Ruuhkaa — yritä hetken päästä uudelleen.", interpretFailed: "Tulkinta epäonnistui.",
+    busy: "Ruuhkaa — yritä hetken päästä uudelleen.", rateLimited: "Liikaa pyyntöjä juuri nyt — hetki ja yritä uudelleen.", interpretFailed: "Tulkinta epäonnistui.",
     modelFailed: "Automaattinen tulkinta ei onnistunut — täytä kentät itse.",
     interpretDone: "Tulkinta valmis — tarkista alta.", connError: "Yhteysvirhe — palvelin ei vastaa.",
     serverDown: "Palvelin ei vastaa — yritetään uudelleen…",
@@ -160,7 +160,7 @@ <h1 id="h1"></h1>
     interpretHeading: "Interpretation — review and correct if needed",
     theme: "Theme", severity: "Severity", type: "Type", cancel: "Cancel", save: "Save",
     writeFirst: "Write the feedback first.", interpreting: "Interpreting…",
-    busy: "Busy — try again in a moment.", interpretFailed: "Interpretation failed.",
+    busy: "Busy — try again in a moment.", rateLimited: "Too many requests right now — wait a moment and retry.", interpretFailed: "Interpretation failed.",
     modelFailed: "Automatic interpretation failed — fill the fields yourself.",
     interpretDone: "Interpretation ready — review below.", connError: "Connection error — the server is not responding.",
     serverDown: "Server not responding — retrying…",
@@ -270,12 +270,23 @@ <h1 id="h1"></h1>
   el("interpret").disabled = true;
   setStatus("status1", "", L.interpreting);
   try {
-    const res = await fetch(`${API}/interpret`, {
-      method: "POST",
-      headers: { "Content-Type": "application/json" },
-      body: JSON.stringify({ text }),
-    });
+    // A 503 is the GPU gate shedding (2 slots busy, ADR-0040) — usually
+    // transient. Retry a FEW times with JITTERED backoff so a roomful of clerks
+    // clicking at once don't re-collide in lockstep; give up to the manual busy
+    // message after that. A 429 is the shared rate bucket (ADR-0025), NOT the
+    // GPU — never hammer it, just tell the user to wait.
+    let res;
+    for (let attempt = 0; ; attempt++) {
+      res = await fetch(`${API}/interpret`, {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: JSON.stringify({ text }),
+      });
+      if (res.status !== 503 || attempt >= 2) break;
+      await new Promise(r => setTimeout(r, 300 + attempt * 400 + Math.random() * 500));
+    }
     if (res.status === 503) { setStatus("status1", "warn", L.busy); return; }
+    if (res.status === 429) { setStatus("status1", "warn", L.rateLimited); return; }
     if (!res.ok) { setStatus("status1", "err", L.interpretFailed); return; }
     const data = await res.json();
     startEntry(text);

tests/FeedbackIntelligence.Api.Tests/LlmGateTests.cs

@@ -0,0 +1,92 @@
+using Microsoft.Extensions.Options;
+using FeedbackIntelligence.Api;
+
+namespace FeedbackIntelligence.Api.Tests;
+
+public class LlmGateTests
+{
+    private static LlmGate Gate(int slots, int acquireMs = 500, int callMs = 120_000) =>
+        new(Options.Create(new IngestOptions
+        {
+            LlmMaxConcurrency = slots,
+            LlmAcquireTimeoutMs = acquireMs,
+            LlmCallTimeoutMs = callMs,
+        }));
+
+    [Fact]
+    public async Task TryRunAsync_RunsWork_WhenASlotIsFree()
+    {
+        var gate = Gate(slots: 2);
+        var ran = false;
+
+        var acquired = await gate.TryRunAsync(_ => { ran = true; return Task.CompletedTask; }, CancellationToken.None);
+
+        Assert.True(acquired); // a free slot means the work runs
+        Assert.True(ran);
+    }
+
+    [Fact]
+    public async Task TryRunAsync_SkipsWork_WhenGateIsSaturated_WithoutBlocking()
+    {
+        // ADR-0040: /health must never add GPU load beyond the gate. Hold BOTH
+        // slots, then a non-blocking probe returns false immediately WITHOUT
+        // running its work and WITHOUT waiting the acquire timeout.
+        var gate = Gate(slots: 2, acquireMs: 10_000); // long acquire — the probe must not wait it
+        var release = new TaskCompletionSource();
+        var occupied = new TaskCompletionSource();
+        var held = 0;
+
+        async Task Hold(CancellationToken _)
+        {
+            if (Interlocked.Increment(ref held) == 2) occupied.SetResult();
+            await release.Task;
+        }
+        var h1 = gate.RunAsync<object?>(async ct => { await Hold(ct); return null; }, CancellationToken.None);
+        var h2 = gate.RunAsync<object?>(async ct => { await Hold(ct); return null; }, CancellationToken.None);
+        await occupied.Task; // both slots now held
+
+        var probeRan = false;
+        var sw = System.Diagnostics.Stopwatch.StartNew();
+        var acquired = await gate.TryRunAsync(_ => { probeRan = true; return Task.CompletedTask; }, CancellationToken.None);
+        sw.Stop();
+
+        Assert.False(acquired);              // gate full — not acquired
+        Assert.False(probeRan);              // ...so the probe work never ran
+        Assert.True(sw.ElapsedMilliseconds < 1_000); // and it did NOT wait the 10 s acquire timeout
+
+        release.SetResult();
+        await Task.WhenAll(h1, h2);
+
+        // Slots freed — a probe now runs again (release really returned them).
+        Assert.True(await gate.TryRunAsync(_ => Task.CompletedTask, CancellationToken.None));
+    }
+
+    [Fact]
+    public async Task TryRunAsync_ReleasesSlot_EvenWhenWorkThrows()
+    {
+        var gate = Gate(slots: 1);
+
+        await Assert.ThrowsAsync<InvalidOperationException>(() =>
+            gate.TryRunAsync(_ => throw new InvalidOperationException("model down"), CancellationToken.None));
+
+        // The single slot must have been released despite the throw — the next
+        // acquire (RunAsync) succeeds rather than shedding.
+        var ran = await gate.RunAsync(_ => Task.FromResult(true), CancellationToken.None);
+        Assert.True(ran);
+    }
+
+    [Fact]
+    public async Task RunAsync_Sheds_WhenNoSlotFreesWithinAcquireTimeout()
+    {
+        var gate = Gate(slots: 1, acquireMs: 50);
+        var release = new TaskCompletionSource();
+        var holding = gate.RunAsync<object?>(async _ => { await release.Task; return null; }, CancellationToken.None);
+        await Task.Delay(20); // let the holder take the only slot
+
+        await Assert.ThrowsAsync<LlmBusyException>(() =>
+            gate.RunAsync(_ => Task.FromResult(true), CancellationToken.None));
+
+        release.SetResult();
+        await holding;
+    }
+}