From 6ae3f54a3634920c8ac4d98073dd9c9030a70579 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Mon, 26 Oct 2026 01:39:01 +0100 Subject: [PATCH] docs: adr 0015 transactional outbox --- CONTRIBUTING.md | 2 + README.md | 2 + docs/adr/0015-transactional-outbox.md | 85 +++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 docs/adr/0015-transactional-outbox.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f4884241..ae8ede8d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -77,6 +77,8 @@ SQLite is the only supported backend (see [ADR-0012](docs/adr/0012-sqlite-is-the ### Federation (ActivityPub) +Events: never publish from a use case after the fact. Pass the events to the repository/command-port method that writes the data so they land in the outbox (`event_queue`) in the same transaction (ADR-0015). The worker delivers them at-least-once and a ledger skips handlers already done, so every handler must be idempotent. + Federation is feature-gated (`#[cfg(feature = "federation")]`). If your feature should federate, add domain events, handle them in `crates/adapters/activitypub/src/event_handler.rs`, and create an AP object + inbound handler. ## Code style diff --git a/README.md b/README.md index 14a75adc..dae48a09 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,8 @@ cargo run -p worker # event worker (poster sync, in a separate terminal) The worker polls the event queue and must run alongside the server to process background tasks like poster fetching. Both processes share the same database. +**Event flow.** The server never publishes events directly: every write that raises an event stores it in the `event_queue` table in the same transaction (a transactional outbox, see [ADR-0015](docs/adr/0015-transactional-outbox.md)). With `EVENT_BUS_BACKEND=db` the worker consumes those rows; with `nats` the worker relays them to JetStream (with `Nats-Msg-Id` set to the event id) and consumes them from there. Duplicates are filtered in three layers: the broker's duplicate window, a claim lease on queue rows, and a per-(event, handler) processed-event ledger. Handlers must stay idempotent. + ## API All REST endpoints are under `/api/v1/`. Authentication uses `Authorization: Bearer ` obtained from `POST /api/v1/auth/login`. diff --git a/docs/adr/0015-transactional-outbox.md b/docs/adr/0015-transactional-outbox.md new file mode 100644 index 00000000..50ae4d24 --- /dev/null +++ b/docs/adr/0015-transactional-outbox.md @@ -0,0 +1,85 @@ +# Events go through a transactional outbox and are delivered idempotently + +Domain events used to be published after the write that caused them, from the use case, on a +channel the server owned. A crash between the write and the publish lost the event (no poster, no +federation delivery, no search index entry), and with NATS the server needed a live broker +connection just to log a review. Delivery was also at-least-once without any guard, so a redelivered +event ran its handlers twice. + +## Decision + +**The outbox is `event_queue`.** The same SQLite table the DB-backed bus already used is now the +outbox for every backend. Migration `0101` adds `event_queue.event_id` (UUID, unique when set) and +`claimed_at`, and creates `processed_events` (`event_id`, `handler`, `status`, `started_at`, owner +token). Legacy rows have a NULL `event_id` and are still delivered; they just get no dedup. + +**Writes are atomic with their events.** Repository and command-port methods that change state take +the events they cause and write them in the same transaction as the data. `shared::enqueue_events` +is the single path that inserts outbox rows. Covered: review log/edit/delete (including orphan +movie deletion and `ReviewDeleted`/`MovieDeleted`), watchlist add/remove, movie discovery and +enrichment requests, wrap-up, integration writes, local follow/unfollow, remote review save. +Use cases never open transactions; the adapter does. + +**The server never talks to NATS.** With `EVENT_BUS_BACKEND=nats` the server still only inserts +outbox rows. The worker runs a relay that claims pending rows and publishes them to JetStream with +`Nats-Msg-Id` set to the event id, then marks the row done. With `db` the worker consumes the rows +directly. `max_deliver` on the JetStream consumer equals `EVENT_QUEUE_MAX_ATTEMPTS`, so both +backends dead-letter after the same number of attempts. + +**Three dedup layers.** +1. *Broker.* JetStream drops a repeated `Nats-Msg-Id` inside `duplicate_window`, set explicitly to 1 + hour (longer than the relay's retry horizon) rather than the 2-minute server default. +2. *Relay lease.* A row claimed by a relay or DB consumer that then died is reclaimed once + `claimed_at` is older than 600 s, so a crash never strands an event in `processing`. +3. *Processed-event ledger.* Before a handler runs, `(event_id, handler)` is claimed in + `processed_events` (INSERT `in_progress`, or take over a claim older than the 1800 s lease). + Success marks it `done`; failure releases it; both are conditioned on an owner token so a + stale worker cannot clobber a newer claim. A delivery that sees a fresh claim held elsewhere is + deferred (`AckHandle::defer`: rescheduled to the lease expiry without consuming an attempt; + JetStream `nak` with delay, which does count toward `max_deliver`). A `done` row skips the handler. + Ledger rows are kept 30 days. A ledger read error skips the handler and nacks. + +## Guarantees + +- No two deliveries run the same handler for the same event concurrently. +- At-least-once after a crash: a lost claim expires and the event is retried. +- Handlers are idempotent, so the remaining duplicate window (crash after the effect, before the + ledger mark) is harmless. The O5 audit made each handler safe to re-run: image cleanup (NotFound + is Ok), image conversion (probes the exact key; missing original means done), follow backfill and + inbound review import (upserts, deterministic Create ids), person/movie enrichment (upserts; the + movie profile write is last because it is the "done" marker), wrap-up (a `Generating` record is + resumed; completion is atomic so `WrapUpCompleted` is emitted once), search handlers (keyed + upserts and removes), poster sync (skips when the poster is already stored under the movie id, so + no second `PosterSynced`/`ImageStored`). + +## Known gaps + +- **Remote and block follow events are best-effort.** `CompositeSocialAdapter` routes remote + follows and blocks through k-ap, which owns that transaction. Its events are published after the + fact through an injected publisher, so a crash between the k-ap write and the publish loses them. + Fixing it needs transaction hooks in k-ap. +- **k-ap 0.5.0 mints random ids for some activities.** Create, Announce and Like ids are + deterministic (`broadcast.rs:22-35`, `:283-288`; backfill `backfill.rs:208-212`). Delete + (`broadcast.rs:157`), Add/Undo (`:210`), Update (`:321`), Move (`:377`) and the actor update + (`:339-344`) use `Uuid::new_v4()` via `url_scheme.rs:52-53` with no override; only + `broadcast_raw_to_followers` (`:395`) takes a caller id. A duplicate of those after a crash is a + fresh-id activity. They are semantically idempotent for remote servers (deleting twice, updating + to the same state), but it is a new activity, not a replay. The ledger makes this rare; closing it + means building those activities ourselves or a k-ap change. +- **Core NATS has no safety net.** `NATS_MODE=core` is fire-and-forget: no broker dedup, no + redelivery. Rows are marked done after publish; a subscriber that is down misses the event. The + ledger still prevents double handling of a repeated id. +- **`ImageStored` stays best-effort.** It is raised after an object-storage write, which no + database transaction can cover. + +## Consequences + +- Existing deployments keep their JetStream stream. `get_or_create_stream` applies config only on + creation, so the consumer now compares an existing stream and consumer with what this build needs + and updates `duplicate_window` (1 h) and the consumer's `max_deliver`/`ack_wait` in place, with a + warning in the log. Both are mutable on a live stream/consumer, so no recreate is needed; + immutable consumer fields (name, filter, deliver policy) are never touched. +- Starting server and worker at the same moment against a database that still needs migration `0101` + can make one fail with `duplicate column name`; start the server first, or restart the loser. +- Handler authors must keep new handlers idempotent; the ledger narrows duplicates but cannot + remove the crash-after-effect case.