fix(worker): provider readiness check + circuit breaker for unreachable Ollama - #448
Merged
Merged
Conversation
…le Ollama Includes a correction: the Sentry issue that prompted this (30 events, "Name or service not known (ollama:11434)", tagged environment: Production) did NOT come from production. It came from a dev machine — linux-arm64 (prod is x86_64), release ee15354 (never deployed), while prod's ollama has been healthy for 4 weeks, prod's worker logged zero ollama lines in 24h, and prod has zero backfill candidates. The "38 starved books" were a local database. The tag lied because the local .env sets ASPNETCORE_ENVIRONMENT=Production. SentryBootstrap now downgrades a Production claim to `production-unverified` when SENTRY_RELEASE is absent (CI images always set it from GIT_SHA). The hardening stands on its own: - AiProviderReadinessCheck (replaces EnrichmentKeyCheck, whose hardcoded provider set had drifted past openai-pdf) probes every referenced provider once at startup — Ollama over HTTP, OpenAI by key presence, never a token — and seeds the circuit open on failure. Cannot crash or block the host. - ProviderCircuitPolicy: pure statics, clock as parameter. OpenedAt survives escalation so JustTripped fires exactly once per outage — that predicate is the whole "1 event, not 30" requirement. Backoff 1/5/30 min. - Detection inside OllamaLlmClient (the only place that sees the wire; the client swallows transport errors so a decorator would see success-with-empty). Non-2xx deliberately does not trip: Ollama answering 404 is alive and cheap. - Batch gates leave rows Pending during an outage — UserBookEnrichmentService stamps Completed (not Failed) on a null result, so claiming them would drain the queue into "done, nothing filled". That is provider-independent data loss. No silent fallback in either direction; ModelGateway.cs is untouched. Verified with Ollama actually down: 38 candidates → 1 HTTP request → abort, 37 stay queued (was: 38 calls × 90s timeout). 1363 unit tests green (79 new). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mrviduus
enabled auto-merge (squash)
August 8, 2026 23:21
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.
First: a correction to the premise
This PR was requested as a fix for a production incident — Sentry issue 7656871760, 30 events,
Name or service not known (ollama:11434), taggedenvironment: Production, "38 books starved".It was not production. Four independent checks:
linux-arm64ee15354c71c6b1da→df551f15→bb7cfc1c→58fec417)ollamacontainerUp 4 weeks (healthy), worker resolves it to172.18.0.8status = Ready AND genre IS NULL)The events came from a developer machine. The "38 starved books" were a local database.
Why the tag lied — and this is the actual bug the episode exposed: the local
.envsetsASPNETCORE_ENVIRONMENT=Production, so a dev box pointed at the production DSN wrote into the production Sentry project under a production tag. Reading that UI, "prod" is the only available conclusion.Changes
1. Stop dev events masquerading as prod.
SentryBootstrap.ResolveEnvironmentNamedowngrades aProductionclaim toproduction-unverifiedwhenSENTRY_RELEASEis absent — the release comes from theGIT_SHAbuild arg in both Dockerfiles, so every CI image has one and nodotnet runever does. Real production unaffected. A rename, not a drop: an unverified event is still worth having, it just must not masquerade. Plus a.env.examplewarning.2. Startup readiness check.
AiProviderReadinessCheckreplacesEnrichmentKeyCheck(strict superset; the old one's hardcoded provider set had already drifted pastopenai-pdf, the most expensive route in the system — a prefix rule now makes that impossible). Probes every provider the route table actually references, once, before any worker loop starts. Ollama viaGET /api/tagson a 2 s timeout — same shape as the API's existing/health/readyprobe. OpenAI-family by key presence only: it never spends a token to prove a paid provider is up. OneLogErrornaming the affected tags, one Sentry warning, circuit seeded open. Cannot crash or block the host.3. Circuit breaker.
ProviderCircuitPolicy— pure statics with the clock as a parameter (this repo has no clock abstraction; mirrorsAlarmThrottle).OpenedAtsurvives escalation soJustTrippedis true exactly once per outage — that single predicate is the entire "one event per condition, not 30" requirement. Backoff 1 → 5 → 30 min. Detection lives insideOllamaLlmClientbecause it is the only place that sees the wire: the client swallows transport failures and returns an empty response, so a decorator would see success-with-empty and could not tell a dead Ollama from a model that said nothing. Non-2xx deliberately does not trip — that means Ollama is alive and answering (usually a wrongOllama:Model→ 404), fails in milliseconds, and burying it under 30 minutes of silence would turn a cheap visible error into an invisible one.4. Batch gates that prevent data loss.
UserBookEnrichmentService.EnrichAsyncstampsCompleted, notFailed, when the generator returns null — exactly what an unreachable provider produces. Claiming rows during an outage therefore drains the queue into a terminal "done, nothing filled" state nothing revisits. The gates leave rowsPendinginstead. This one is provider-independent and was worth the trip on its own.No silent fallback in either direction.
ModelGateway.csis not modified at all. Rerouting an outage onto a paid provider is the 2026-07-14 390%-CPU incident with the arrow reversed.Before / after
Completed, nothing filledPending, picked up on recoveryVerified against the real failure, not a mock
Local Ollama stopped, Worker started with
SENTRY_DSNunset:Exactly one HTTP request reached the wire; the 38 candidate rows were still queued afterwards.
That per-book re-check exists because the first live run exposed a gap in my own design: the startup probe opens the circuit on a 1-minute rung while the backfill only wakes at its 2-minute start delay, so the circuit is legitimately half-open by then and a single up-front gate let the whole batch begin.
1363 unit tests green (79 new), build +
dotnet format --verify-no-changesclean. No migration.Rollback plan
Ai:ProviderHealth:Enabled=falserestores byte-for-byte the previous behaviour without a deploy. Full revert is a cleangit revert— no migration, no schema, no data.Notes
/health/ready, and it inherits the breaker viaAddApplication()).🤖 Generated with Claude Code