From 3d9269e54fad9d9d0cbc3ea3baf7bbab1a716df4 Mon Sep 17 00:00:00 2001 From: Ali Zarinkolah Date: Thu, 20 Aug 2026 22:26:58 +0330 Subject: [PATCH] docs(ops): add the operator runbook and record Phase 6 as complete Why: - the slice had no operator documentation: no tuning guidance, no failure procedure, no statement of the proxy timeout requirement. Changes: - add docs/runbook.md: startup, deployment steps, provisioning, the INGESTION_* tuning table with each bound's status code, the proxy read-timeout rule, /healthz vs /readyz, failure investigation by real event name plus job/event SQL, retry semantics, and alert thresholds - link it from the README and note provisioning there - mark plan 001 Phase 6 done and refresh CLAUDE.md's status paragraph Impact: - documentation only Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 24 +- README.md | 15 ++ docs/plans/001-ingestion-vertical-slice.md | 19 +- docs/runbook.md | 291 +++++++++++++++++++++ 4 files changed, 339 insertions(+), 10 deletions(-) create mode 100644 docs/runbook.md diff --git a/CLAUDE.md b/CLAUDE.md index 155c45c..58a2f68 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,9 +27,16 @@ Also working: `tenant_domains` plus `/v1/domains` (`src/application/domains/`), a strict per-tenant allowlist — `POST /v1/files` rejects an unregistered or disabled `domain` with `400` before anything is written, and domain management sits behind its own `domains:read`/`domains:write` scopes, never `files:write`. -Not built yet: `/v1/points` CRUD and keyword search (plan 002), and -`src/agent/`. That maps to plan 001 Phases 1-5 done, Phase 6 (runbook, e2e -tests, Compose smoke test) not started. +Also working: the operator runbook (`docs/runbook.md`), tenant/API-key/domain +provisioning (`uv run python -m src.cli.provision_tenant` — the third deployment +step, since nothing over HTTP can create the first tenant), a Testcontainers +e2e suite in the default pytest run (`tests/e2e/test_ingestion_slice.py`: +duplicate upload, retry after failure, tenant isolation, capacity, timeout, +parse and Qdrant failure), and the one Compose-based test — `scripts/smoke.sh` +driving `tests/e2e/test_compose_smoke.py` against a real uvicorn process, which +skips itself unless `SMOKE_BASE_URL` is set. Not built yet: `/v1/points` CRUD +and keyword search (plan 002), and `src/agent/`. That maps to plan 001 Phases +1-6 done. Architecture decisions live in `docs/adr/` (18 ADRs plus the 0000 template; 0001–0004 are `Accepted` — 0004 amended by 0018; 0014 is `Superseded by 0017`; @@ -285,9 +292,14 @@ belongs, not the row-level function underneath it. - Test naming: `test___`, Arrange–Act–Assert. - Layout mirrors architecture: `tests/unit/{application,agent}`, `tests/integration/{postgres,minio,qdrant}`, `tests/e2e/`. -- Integration tests use **Testcontainers** (never a developer's local - services or Langfuse-owned storage/credentials) — this is the standard - automated mechanism, not Docker Compose. Isolate data per test via unique +- Integration **and e2e** tests use **Testcontainers** (never a developer's + local services or Langfuse-owned storage/credentials) — this is the standard + automated mechanism, not Docker Compose. Compose is reserved for exactly one + thing: the serialized operational smoke test of the *running web process* + (`scripts/smoke.sh`), which is gated out of `uv run pytest`. Shared container + fixtures live in `tests/support/containers.py`, registered from the root + `tests/conftest.py` via `pytest_plugins` (a non-root conftest cannot declare + it). Isolate data per test via unique keys/queue/collection names; parallel integration execution is disabled until fixture isolation is proven safe. - Pytest never calls a live/paid model provider in routine runs — that's diff --git a/README.md b/README.md index 3d851c6..4f8d872 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,9 @@ Architecture decisions live in [`docs/adr`](docs/adr). The first implementation milestone is documented in the [ingestion vertical-slice plan](docs/plans/001-ingestion-vertical-slice.md). +Day-to-day operation — tuning the ingestion bounds, the proxy timeout +requirement, and how to investigate or retry a failed upload — is the +[operator runbook](docs/runbook.md). ## Provisioning the datastores @@ -16,6 +19,14 @@ uv run python -m src.cli.qdrant_bootstrap # the `chunks` collection uv run fastapi dev src/main.py ``` +Nothing over HTTP can create the first tenant — every `/v1` route needs an API +key, and a key cannot exist before its tenant. One command issues both, plus any +domains, printing the key once (only its hash is stored): + +```bash +uv run python -m src.cli.provision_tenant --slug acme --domain fire +``` + Before a tenant can upload, its domains must be registered — `POST /v1/files` rejects an unregistered or disabled `domain` with `400`. The calling backend manages them over `/v1/domains` using a key with the `domains:write` scope: @@ -31,6 +42,10 @@ Both bootstrap commands are idempotent and safe to re-run. `qdrant_bootstrap` ve existing collection against the pinned schema and exits non-zero on a mismatch, rather than leaving a silently degraded sparse index in place. +`./scripts/smoke.sh` verifies the whole path — Compose up, both deployment +steps, provisioning, an upload through the running web process to indexed Qdrant +points. See the [runbook](docs/runbook.md#12-verifying-a-deployment). + ## Local Langfuse This repo includes a root-level development Compose file for Langfuse: diff --git a/docs/plans/001-ingestion-vertical-slice.md b/docs/plans/001-ingestion-vertical-slice.md index 4c6e387..2df558a 100644 --- a/docs/plans/001-ingestion-vertical-slice.md +++ b/docs/plans/001-ingestion-vertical-slice.md @@ -289,17 +289,28 @@ retrying the upload produces a correct final state without duplicate chunks. 1. Add an operator runbook covering local startup, migrations, MinIO bucket setup, the run command, ingestion-bound tuning, the proxy/client timeout - requirement, and how to retry a failed ingestion. + requirement, and how to retry a failed ingestion. — `docs/runbook.md`. 2. Add a serialized Compose-based operational smoke test covering upload through indexed points against the running web process. Testcontainers remains the - standard pytest mechanism for individual adapter integration tests. + standard pytest mechanism for individual adapter integration tests. — + `scripts/smoke.sh` driving `tests/e2e/test_compose_smoke.py`, which skips + itself unless `SMOKE_BASE_URL` is set so `uv run pytest` never invokes + Compose. 3. Add end-to-end tests for duplicate upload, retrying a failed upload, tenant - isolation, capacity/timeout rejection, and failed parser/Qdrant behavior. + isolation, capacity/timeout rejection, and failed parser/Qdrant behavior. — + `tests/e2e/test_ingestion_slice.py`, on Testcontainers, in the default suite. 4. Add health/readiness checks that distinguish process health from dependency - readiness. + readiness. — `/healthz` and `/readyz`; `/readyz` additionally requires the + `chunks` collection to exist, since a reachable but unbootstrapped Qdrant + would `502` on the first upload. 5. Update the README with local-start instructions and links to ADRs, this plan, and the operations runbook. +Provisioning a tenant and its first API key turned out to be a prerequisite for +1 and 2 rather than a separate milestone: nothing over HTTP can create the first +tenant, so `src/cli/provision_tenant.py` was added alongside the other two +deployment-step commands. + **Exit criteria:** a new developer can start the stack, apply migrations, upload a a document, observe the job through completion, and understand how to investigate or retry a failure. diff --git a/docs/runbook.md b/docs/runbook.md new file mode 100644 index 0000000..dd97cf7 --- /dev/null +++ b/docs/runbook.md @@ -0,0 +1,291 @@ +# Operator runbook + +How to start this service, configure its ingestion bounds, and investigate or +retry a failed upload. Architecture rationale lives in [`docs/adr/`](adr/); the +implementation milestone is +[plan 001](plans/001-ingestion-vertical-slice.md). This document covers +operating what those describe. + +The service is a **single process with no background work**. `POST /v1/files` +parses, chunks, embeds, and indexes inline and returns a terminal result +(ADR-0017). There is no queue, no worker, and no automatic retry — the caller +owns the retry decision, which makes the request's duration a deployment +constraint. That fact drives most of this document. + +## 1. Prerequisites and local startup + +Docker, and [`uv`](https://docs.astral.sh/uv/) with Python 3.13. + +```bash +cp .env.example .env # non-secret local defaults; .env is gitignored +uv sync +docker compose up -d --wait +``` + +`docker-compose.yml` runs Postgres (`127.0.0.1:5433`), MinIO +(`127.0.0.1:9100`, console `9101`), and Qdrant (`127.0.0.1:6343`). It is the +local development stack and says so in its header — it is not a production +deployment. + +## 2. Deployment steps + +Two schema steps run **before** the application, never at startup: FastAPI +performs no DDL, for Postgres (ADR-0009) or for Qdrant (ADR-0001, "Collection +provisioning"). Both commands and their reasoning are in the README's +[Provisioning the datastores](../README.md#provisioning-the-datastores) +section: + +```bash +uv run alembic upgrade head # Postgres schema +uv run python -m src.cli.qdrant_bootstrap # the `chunks` collection +``` + +Both are idempotent. `qdrant_bootstrap` verifies an existing collection against +the pinned schema and **exits non-zero on a mismatch** rather than leaving a +silently degraded sparse index in place — the `sparse` vector's +`modifier="idf"` and the pinned dense dimensions (768 / 3072) fail silently if +wrong, which is why they are checked rather than assumed. + +Run both again after every deploy that ships a migration or a collection-schema +change. + +## 3. MinIO bucket + +Under Compose the bucket already exists: the `app-minio` service's entrypoint +runs `mkdir -p /data/${MINIO_BUCKET:-chatbot-source-files}` before starting the +server, so first boot creates it. Nothing else needs to be done locally. + +Outside Compose, create the bucket named by `MINIO_BUCKET` before the first +upload — the application never creates it. It must stay **private**; ADR-0013 +keeps source bytes non-public and this slice ships no download API or presigned +URLs. + +## 4. Provisioning a tenant, an API key, and its domains + +Nothing over HTTP can bootstrap a tenant: every `/v1` route needs an API key, +and a key cannot exist before its tenant. So the first key is issued by an +operator command: + +```bash +uv run python -m src.cli.provision_tenant \ + --slug acme --domain fire --domain life --scopes files:write,domains:read +``` + +It prints `api_key=sk_...` **once**. Postgres stores only its SHA-256 hash +(ADR-0009), so a lost key is reissued by re-running the command, never +recovered. Structured logs carry only the non-secret `key_prefix` — a plaintext +key must never reach a log sink (ADR-0011). + +Re-running with the same `--slug` reuses the tenant and any domains it already +has, and issues an **additional** key. Both keys stay valid; this adds a key, it +does not rotate one. + +Scopes are the security boundary between uploading and managing the allowlist. +Give an upload client `files:write` only. `domains:write` lets its holder create +new domains, which is exactly what the allowlist exists to prevent an upload key +from doing. + +### Domains after the first one + +`POST /v1/files` rejects an unregistered or disabled `domain` with `400` +(`unknown_domain`) before anything is written. Ongoing domain management is the +`/v1/domains` API, under `domains:read` / `domains:write`: + +```bash +curl -X POST http://localhost:8000/v1/domains \ + -H "Authorization: Bearer $API_KEY" -H 'Content-Type: application/json' \ + -d '{"domain": "fire", "display_name": "Fire insurance"}' +``` + +The `domain` key itself is immutable — it is denormalized into every Qdrant +point payload and into `source_files`, so renaming it is a migration, not an +edit (ADR-0009). Disabling a domain blocks new uploads; it does not delete +existing points. + +## 5. Running the service + +```bash +uv run fastapi dev src/main.py # local, reload +uv run uvicorn src.main:app --host 0.0.0.0 --port 8000 # deployed shape +``` + +Run more than one worker/replica only after reading §6: ingestion bounds are +**per process**, so `INGESTION_MAX_CONCURRENCY` multiplies by the number of +processes. + +## 6. Ingestion bounds and tuning + +Every bound is enforced server-side and maps to a status code. All are in +`.env.example`. Ingestion is CPU- and network-bound in the request, so these are +the numbers that decide whether the service degrades gracefully or falls over. + +| Setting | Bounds | On breach | Size it against | +|---|---|---|---| +| `INGESTION_MAX_CONCURRENCY` | Ingestions in flight **per process** | `503` + `Retry-After` | Memory per in-flight upload (whole file plus its chunks and vectors are resident) and the embedder's capacity. Rejecting is deliberate: ADR-0017 refuses rather than queues. | +| `INGESTION_THREAD_POOL_SIZE` | Threads for blocking work (parse, chunk, hash, BM25, the sync `minio` SDK) | — (waits) | CPU cores. It exists to stop ingestion exhausting Starlette's own thread pool, so keep it below the total thread budget. | +| `INGESTION_TIMEOUT_SECONDS` | The whole work phase | `504`, job marked `failed` | The slowest legitimate document, plus headroom. See §7 — this must stay under every read timeout in front of it. | +| `INGESTION_MAX_UPLOAD_SIZE_MB` | Bytes accepted | `413` | Memory: the upload is read fully into the process before any work starts. | +| `INGESTION_MAX_CHUNKS_PER_FILE` | Chunks per file, checked before embedding | `413` | Embedder cost/time per chunk × `INGESTION_TIMEOUT_SECONDS`. This is the real defence against one pathological file eating a slot. | +| `INGESTION_EMBED_BATCH_SIZE` | Texts per embedder request | `502` on embedder failure | The provider's per-request limits. Batch before parallelizing. | +| `INGESTION_EMBED_CONCURRENCY` | Concurrent embed batches | `502` | Provider rate limits and the self-hosted embedder's throughput. Never unbounded. | +| `QDRANT_UPSERT_BATCH_SIZE` / `_CONCURRENCY` | Points per upsert and concurrent upserts | `502` (`index_error`) | Qdrant's ingest capacity; the batch size stays in ADR-0001's 64–256 band. | + +Two settings that look like tuning knobs but are not: + +- **`EMBEDDING_NOMIC_KEEP_ALIVE`** holds the self-hosted model resident. A cold + load of `nomic-embed-text-v2-moe` takes over 150 s — longer than any sane + `INGESTION_TIMEOUT_SECONDS` — so an idle period followed by an upload would + otherwise `504`. The lifespan also warms both dense embedders at startup for + the same reason. +- **The BM25 analyzer and weights** (`EMBEDDING_SPARSE_*`) are a measured + artifact ported from the `emet` evaluation lab, verified token-for-token + against it (ADR-0005). Re-benchmark; do not tune them in place. + +## 7. The proxy and client read-timeout requirement + +**Every read timeout in front of this service must exceed +`INGESTION_TIMEOUT_SECONDS`.** That includes the reverse proxy / ingress, any +load balancer, and the calling backend's own HTTP client. + +If a proxy times out first, the client gets that proxy's error, the upload keeps +running in the process, and the caller learns nothing about the outcome from the +response. The job row still reaches a terminal status, so +`GET /v1/files/{file_id}` remains the way to find out what happened — but the +response contract is broken for that request. ADR-0017 names this the main cost +of inline ingestion. + +A workable local ordering: client read timeout > proxy read timeout > +`INGESTION_TIMEOUT_SECONDS`. + +## 8. Health and readiness + +| Endpoint | Question it answers | Use for | +|---|---|---| +| `GET /healthz` | Is the process alive? | Liveness probes / restart policy. Never depends on Postgres, MinIO, or Qdrant. | +| `GET /readyz` | Can it actually serve? | Load-balancer admission and post-deploy gating. `200` with each dependency `true`, `503` if any is `false`. | + +`/readyz` checks Postgres, MinIO, and Qdrant reachability **and** that the +`chunks` collection exists. A reachable-but-unbootstrapped Qdrant reports +`{"qdrant": false}` on purpose: uploads to it would fail with `502`, so it is +not ready, and this is how a skipped `qdrant_bootstrap` surfaces at deploy time +instead of on a user's first upload. + +## 9. Investigating a failure + +Logs are structured (`structlog`, JSON in production) with stable event names — +grep the event name, not prose (ADR-0011). Set `LOG_FILE_PATH` for a local +JSON file sink alongside the console renderer; leave it unset in production, +where stdout collection is preferred. + +**Correlate by `request_id`.** Every request has one, echoed in the +`X-Request-Id` response header and included in every error envelope, and bound +into every log line emitted while handling that request. A client reporting a +failed upload should quote it. `tenant_id`, `file_id`, and `ingestion_job_id` +are the other join keys. + +Events worth knowing: + +| Event | Level | Means | +|---|---|---| +| `ingestion.job.started` | info | Txn A committed; work phase beginning. Carries `tenant_id`, `ingestion_job_id`, `file_id`, `domain`, `source_type`. | +| `ingestion.job.completed` | info | Terminal success, with `chunks_parsed`, `points_upserted`, `points_soft_deleted`. | +| `ingestion.job.failed` | warning | Terminal failure. **`error_code` says which stage**: `storage_upload_failed`, `parse_failed`, `chunk_limit_exceeded`, `embedding_failed`, `index_failed`, `timeout`. | +| `files.upload.duplicate` | info | Identical content already ingested; the existing file/job was returned and nothing was re-ingested. | +| `domain.rejected` | warning | Upload refused before any row was written; `reason` is `unregistered` or `disabled`. | +| `auth.failed` | warning | `reason` is `malformed_key`, `unknown_key`, `key_inactive`, `key_expired`, or `tenant_inactive`. Never contains key material. | +| `auth.succeeded` | info | Carries `tenant_id`, `api_key_id`, `actor_type`. | +| `lifespan.embedder.warm_failed` | warning | An embedder was unreachable at startup. Boot continues by design — `/readyz` and the first upload are where this bites. | +| `qdrant.bootstrap.schema_mismatch` | error | The existing collection diverges from the pinned schema. The bootstrap exits non-zero; do not start the app against it. | +| `api.unhandled_exception` | error | A bug: an exception with no mapping to the error envelope. Always worth a look. | + +A `503` (`ingestion_at_capacity`) is rejected before a job row exists, so it +appears in the access log and metrics, not in `ingestion_jobs`. + +### The durable record + +Logs may roll; `ingestion_jobs` and `ingestion_job_events` do not. For one file: + +```sql +SELECT id, status, error_code, error_message, points_created, points_soft_deleted, + created_at, updated_at +FROM ingestion_jobs +WHERE tenant_id = :tenant_id AND source_file_id = :file_id +ORDER BY created_at DESC; + +SELECT stage, level, message, details, created_at +FROM ingestion_job_events +WHERE tenant_id = :tenant_id AND ingestion_job_id = :ingestion_job_id +ORDER BY created_at; +``` + +Recent failures across a tenant: + +```sql +SELECT error_code, count(*), max(created_at) +FROM ingestion_jobs +WHERE tenant_id = :tenant_id AND status = 'failed' AND created_at > now() - interval '1 day' +GROUP BY error_code ORDER BY 2 DESC; +``` + +`GET /v1/files/{file_id}` reports the same terminal status over HTTP, scoped to +the owning tenant — a file belonging to another tenant returns `404`, not `403`. + +## 10. Retrying a failed ingestion + +**Re-upload the same bytes.** There is no retry endpoint and no automatic retry; +the client owns that decision (ADR-0017). + +What that guarantees: + +- Identical content with a **succeeded** job is recognized by + `(tenant_id, domain, content_sha256)` and returned as-is with `200` — no + re-ingestion, no duplicate points. +- Identical content whose last job **failed** starts a fresh job against the + same `source_files` row. A terminal job is never moved back to `running`. +- Point ids are deterministic from `file_id` + `chunk_index` (ADR-0001), so the + retry **overwrites in place** — it cannot duplicate chunks. +- A failed attempt never empties or partially deletes a working index: the + soft-delete sweep that retires a shortened file's leftover points runs only + after every upsert has succeeded. An interrupted attempt can leave a prefix + updated; a retry converges (ADR-0017, "Re-running an ingestion stays safe"). + +Fix the cause first — the `error_code` says where to look: + +| `error_code` | Usual cause | +|---|---| +| `parse_failed` | The file is corrupt or is not really the type its extension claims. Retrying identical bytes will fail identically. | +| `chunk_limit_exceeded` | The file is genuinely too large for one inline ingestion. Split it, or raise `INGESTION_MAX_CHUNKS_PER_FILE` knowing what §6 says about the timeout. | +| `embedding_failed` | The embedder is down, rate-limiting, or unauthenticated. Fix it, then retry — this one usually succeeds unchanged. | +| `index_failed` | Qdrant is down, or the collection is missing (run `qdrant_bootstrap`). | +| `timeout` | The work exceeded `INGESTION_TIMEOUT_SECONDS`. Check whether the embedder was cold (see `lifespan.embedder.warm_failed` and `KEEP_ALIVE`) before raising the bound. | +| `storage_upload_failed` | MinIO is unreachable or the bucket is missing (§3). | + +## 11. What to alert on + +ADR-0017's own triggers for moving ingestion back off the request path. These +are the numbers that say the inline design has stopped fitting: + +- **p95 ingestion duration** approaching `INGESTION_TIMEOUT_SECONDS`. +- **`503` and `504` rates** ceasing to be negligible. +- **Jobs stuck in `running` past the timeout** — every handled failure writes a + terminal status, so a non-zero count here means the process died mid-request: + + ```sql + SELECT count(*) FROM ingestion_jobs + WHERE status = 'running' AND created_at < now() - interval '5 minutes'; + ``` + +Also worth alerting: any `qdrant.bootstrap.schema_mismatch`, a sustained +`/readyz` `503`, and any `api.unhandled_exception`. + +## 12. Verifying a deployment + +```bash +./scripts/smoke.sh +``` + +Brings up Compose, runs both deployment steps, provisions a throwaway tenant, +starts the web process, and drives an upload through to indexed Qdrant points +against the **running process** — including asserting the structured log output +from §9. It is the only Compose-based test; everything else runs on +Testcontainers under `uv run pytest` (ADR-0016). Run it before a release.