diff --git a/.env.example b/.env.example index 4ce7cf73..893dbd32 100644 --- a/.env.example +++ b/.env.example @@ -318,6 +318,22 @@ INROAD_RUN_SCHEDULER=true # the scheduler regardless of INROAD_RUN_SCHEDULER. INROAD_WORKER_ROLE= +# --- Data retention (maintenance:retention, hourly, on the scheduling worker) -- +# Whole DAYS; 0 or blank disables that table's sweep. The four windows over +# the five tables that identify the people your workspaces email (sends, inbox +# threads + messages, tracking events, deliverability events) are DISABLED by +# default on purpose: +# how long to keep that data is a Privacy/Legal decision for your deployment +# (applicable privacy law, your contracts), not something Inroad can pick for +# you. Read "Data retention" in the environment-variable reference before +# enabling any of them -- each has a minimum the worker enforces at startup, and +# each has consequences for reporting. Dead letters keep their 90-day default. +INROAD_RETENTION_SENDS_DAYS= +INROAD_RETENTION_INBOX_DAYS= +INROAD_RETENTION_TRACKING_EVENTS_DAYS= +INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS= +INROAD_RETENTION_DEAD_LETTERS_DAYS= + # --- Fleet credential broker ------------------------------------------------- # Only for a multi-host (role=send) deployment. Leave every one of these blank # for the single-process self-host topology; none of them exists for you. diff --git a/cmd/worker/jobrunledger_test.go b/cmd/worker/jobrunledger_test.go index 853dc5e5..e46e85ac 100644 --- a/cmd/worker/jobrunledger_test.go +++ b/cmd/worker/jobrunledger_test.go @@ -27,6 +27,7 @@ var scheduledSweeps = map[string]string{ queue.TaskInboxSweep: jobrun.NameInboxSweep, queue.TaskWarmupSweep: jobrun.NameWarmupSweep, queue.TaskMaintenanceCleanup: jobrun.NameMaintenanceCleanup, + queue.TaskMaintenanceRetention: jobrun.NameRetention, queue.TaskDomainAuthSweep: jobrun.NameDomainAuthSweep, queue.TaskRecipientESPSweep: jobrun.NameRecipientESPSweep, queue.TaskFleetRotate: jobrun.NameFleetRotate, @@ -40,9 +41,9 @@ var scheduledSweeps = map[string]string{ // loudly if one were reached, which is the assertion: a periodic reconcile // should do nothing here but be counted. The methods that ARE overridden are // exactly the sweeps' entry points, each answering "nothing due", plus the -// four capability interfaces Register resolves by type assertion -// (maintenance.Cleaner, recipientesp.Core, fleet.Rotator and — reached via -// inbox.RegisterScheduled — inbox.PendingSweepCore). Without those the handlers +// five capability interfaces Register resolves by type assertion +// (maintenance.Cleaner, maintenance.Retainer, recipientesp.Core, fleet.Rotator +// and — reached via inbox.RegisterScheduled — inbox.PendingSweepCore). Without those the handlers // are never registered at all and ProcessTask would report "handler not found", // which is itself a failure this test should catch. type recordingCore struct { @@ -79,7 +80,6 @@ func (c *recordingCore) CleanupExpired(context.Context) (int64, error) func (c *recordingCore) PurgeIdempotencyKeys(context.Context) (int64, error) { return 0, nil } func (c *recordingCore) PurgeWarmupObservations(context.Context) (int64, error) { return 0, nil } func (c *recordingCore) PurgeDeadWorkers(context.Context) (int64, error) { return 0, nil } -func (c *recordingCore) PurgeDeadLetters(context.Context) (int64, error) { return 0, nil } func (c *recordingCore) PurgeWebhookDeliveries(context.Context) (int64, error) { return 0, nil } func (c *recordingCore) PurgeScheduledJobRuns(context.Context) (int64, error) { return 0, nil } func (c *recordingCore) PurgeWorkerProviderSignals(context.Context) (int64, error) { @@ -87,6 +87,37 @@ func (c *recordingCore) PurgeWorkerProviderSignals(context.Context) (int64, erro } func (c *recordingCore) PurgeFleetDecisions(context.Context) (int64, error) { return 0, nil } +// --- maintenance.Retainer (resolved by type assertion in Register) --- +// +// Each answers "nothing aged out". The Deps below enable every table, so the +// retention handler really does call each of these — a sweep that registered +// but skipped every table would record a row while exercising nothing. + +func (c *recordingCore) PurgeDeliverabilityEvents(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return coreapi.RetentionBatch{}, nil +} +func (c *recordingCore) PurgeInboxThreads(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return coreapi.RetentionBatch{}, nil +} +func (c *recordingCore) RollupTrackingEvents(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return coreapi.RetentionBatch{}, nil +} +func (c *recordingCore) PurgeSends(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return coreapi.RetentionBatch{}, nil +} +func (c *recordingCore) PurgeDeadLetters(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return coreapi.RetentionBatch{}, nil +} +func (c *recordingCore) LoadRetentionProgress(context.Context, string) (coreapi.RetentionProgress, error) { + return coreapi.RetentionProgress{}, nil +} +func (c *recordingCore) SaveRetentionProgress(context.Context, string, coreapi.RetentionCursor, bool) error { + return nil +} +func (c *recordingCore) TryRetentionSweepLock(context.Context) (func() error, bool, error) { + return func() error { return nil }, true, nil +} + // --- recipientesp.Core (resolved by type assertion in Register) --- func (c *recordingCore) PurgeExpiredRecipientDomains(context.Context, time.Duration) (int64, error) { @@ -113,6 +144,7 @@ var ( _ coreapi.Client = (*recordingCore)(nil) _ jobrun.Recorder = (*recordingCore)(nil) _ maintenance.Cleaner = (*recordingCore)(nil) + _ maintenance.Retainer = (*recordingCore)(nil) _ recipientesp.Core = (*recordingCore)(nil) _ fleet.Rotator = (*recordingCore)(nil) _ workerinbox.PendingSweepCore = (*recordingCore)(nil) @@ -141,7 +173,11 @@ func TestEverySweepDispatchedThroughRegisterRecordsOneLedgerRow(t *testing.T) { // nil senders/readers/resolvers/enqueuer: a sweep with nothing due reaches // none of them, and a nil dereference here would mean a reconcile did more // than scan-and-count. jobrun.Record re-panics, so it would fail loudly. - worker.Register(mux, worker.Deps{Role: worker.RoleAll, Core: core, PublicURL: "https://app.test"}) + worker.Register(mux, worker.Deps{Role: worker.RoleAll, Core: core, PublicURL: "https://app.test", + Retention: maintenance.RetentionPolicy{ + DeliverabilityEvents: 90 * 24 * time.Hour, InboxThreads: 30 * 24 * time.Hour, + TrackingEvents: 30 * 24 * time.Hour, Sends: 90 * 24 * time.Hour, DeadLetters: 90 * 24 * time.Hour, + }}) for taskType, wantName := range scheduledSweeps { t.Run(taskType, func(t *testing.T) { diff --git a/cmd/worker/main.go b/cmd/worker/main.go index c4f43bdd..202d8962 100644 --- a/cmd/worker/main.go +++ b/cmd/worker/main.go @@ -38,6 +38,7 @@ import ( "github.com/inroad/inroad/internal/platform/warmup" "github.com/inroad/inroad/internal/worker" "github.com/inroad/inroad/internal/worker/fleetsignal" + "github.com/inroad/inroad/internal/worker/maintenance" ) // workerHeartbeatInterval is how often a worker refreshes its `workers` row. It @@ -125,6 +126,19 @@ func run() error { return err } + // The retention windows are validated against their per-table floors here, + // before anything connects, for the same reason as the role: a window short + // enough to delete rows a live rate still reads must stop the process, not + // start deleting. Logged either way, because "is retention on here, and for + // what" should be answerable from the startup log. + retention, err := retentionPolicy(cfg) + if err != nil { + logger.Error("invalid retention configuration", "err", err) + return err + } + logger.Info("retention", "enabled_days", retention.Enabled(), + "note", "recipient-data tables are disabled unless INROAD_RETENTION_*_DAYS is set; see the environment-variable reference") + // Prometheus /metrics listener. mtx is always constructed (never nil): the // campaign/warmup send handlers' finalize points record into it // unconditionally below; INROAD_METRICS_ADDR only controls whether the @@ -447,6 +461,7 @@ func run() error { TrackingSecret: cfg.TrackingSecret, WarmupSecret: cfg.WarmupSecret, WebhookAllowPrivate: cfg.WebhookAllowPrivate, + Retention: retention, Metrics: mtx, }) @@ -458,6 +473,23 @@ func run() error { return nil } +// retentionPolicy builds the retention sweep's policy from configuration and +// refuses any window below its table's floor. Split out so the mapping — which +// variable feeds which table — is testable without starting a worker. +func retentionPolicy(cfg *config.Config) (maintenance.RetentionPolicy, error) { + p := maintenance.RetentionPolicyFromDays(maintenance.RetentionDays{ + DeliverabilityEvents: cfg.RetentionDeliverabilityEventsDays, + InboxThreads: cfg.RetentionInboxDays, + TrackingEvents: cfg.RetentionTrackingEventsDays, + Sends: cfg.RetentionSendsDays, + DeadLetters: cfg.RetentionDeadLettersDays, + }) + if err := p.Validate(); err != nil { + return maintenance.RetentionPolicy{}, err + } + return p, nil +} + // resolveWorkerQueues decides the queue set this process consumes. An operator // override (INROAD_WORKER_QUEUES, surfaced as cfg.WorkerQueues) wins ENTIRELY // when present — an operator who names queues means exactly those, and diff --git a/cmd/worker/retention_test.go b/cmd/worker/retention_test.go new file mode 100644 index 00000000..c0505c9d --- /dev/null +++ b/cmd/worker/retention_test.go @@ -0,0 +1,58 @@ +package main + +import ( + "strings" + "testing" + "time" + + "github.com/inroad/inroad/internal/platform/config" + "github.com/inroad/inroad/internal/worker/maintenance" +) + +const day = 24 * time.Hour + +// The composition root is the ONLY producer of worker.Deps.Retention, whose zero +// value disables every table including dead letters. So this pins that the real +// mapping from configuration does not silently produce that zero value: with +// configuration at its defaults, dead letters keep their 90 days and every +// recipient-identifying table stays off. +func TestRetentionPolicyFromDefaultConfigKeepsDeadLetterRetention(t *testing.T) { + p, err := retentionPolicy(&config.Config{RetentionDeadLettersDays: config.DefaultRetentionDeadLettersDays}) + if err != nil { + t.Fatalf("retentionPolicy: %v", err) + } + want := maintenance.RetentionPolicy{DeadLetters: 90 * day} + if p != want { + t.Fatalf("policy = %+v, want %+v", p, want) + } +} + +// Each variable feeds its own table. A transposed pair here would pass every +// config test and apply, say, the sends window to deliverability events. +func TestRetentionPolicyMapsEachSettingToItsTable(t *testing.T) { + p, err := retentionPolicy(&config.Config{ + RetentionDeliverabilityEventsDays: 100, RetentionInboxDays: 31, RetentionTrackingEventsDays: 32, + RetentionSendsDays: 101, RetentionDeadLettersDays: 8, + }) + if err != nil { + t.Fatalf("retentionPolicy: %v", err) + } + want := maintenance.RetentionPolicy{ + DeliverabilityEvents: 100 * day, InboxThreads: 31 * day, TrackingEvents: 32 * day, + Sends: 101 * day, DeadLetters: 8 * day, + } + if p != want { + t.Fatalf("policy = %+v, want %+v", p, want) + } +} + +// Below a floor, the worker must refuse to start — and say which setting. +func TestRetentionPolicyRefusesAWindowBelowItsFloor(t *testing.T) { + _, err := retentionPolicy(&config.Config{RetentionSendsDays: 30}) + if err == nil { + t.Fatal("a 30-day sends window was accepted; the floor is 90") + } + if !strings.Contains(err.Error(), "INROAD_RETENTION_SENDS_DAYS") { + t.Fatalf("error %q does not name the setting", err) + } +} diff --git a/cmd/worker/scheduler.go b/cmd/worker/scheduler.go index 90ab654a..ad42b817 100644 --- a/cmd/worker/scheduler.go +++ b/cmd/worker/scheduler.go @@ -32,6 +32,7 @@ func sweepRegistrars() []sweepRegistrar { {jobrun.NameInboxSweep, queue.RegisterInboxSweep}, {jobrun.NameWarmupSweep, queue.RegisterWarmupSweep}, {jobrun.NameMaintenanceCleanup, queue.RegisterMaintenanceCleanup}, + {jobrun.NameRetention, queue.RegisterMaintenanceRetention}, {jobrun.NameDomainAuthSweep, queue.RegisterDomainAuthSweep}, {jobrun.NameRecipientESPSweep, queue.RegisterRecipientESPSweep}, {jobrun.NameFleetRotate, queue.RegisterFleetRotate}, diff --git a/docs/src/content/docs/deploy/environment-variables.md b/docs/src/content/docs/deploy/environment-variables.md index bbda8776..0787e08c 100644 --- a/docs/src/content/docs/deploy/environment-variables.md +++ b/docs/src/content/docs/deploy/environment-variables.md @@ -7,7 +7,7 @@ description: Every environment variable the Inroad backend reads, with its real the only place the binaries read `INROAD_*` configuration from — the one exception is `INROAD_LOG_LEVEL`, which `internal/platform/log` reads directly (and alone) so a logger can exist before config does. Every default below is -that file's, and the tables are complete against it: all **82** variables the +that file's, and the tables are complete against it: all **87** variables the backend reads are listed here. ## How values are parsed @@ -362,10 +362,11 @@ plus headroom for the periodic sweepers and HTTP handlers — see ### The scheduler must be a singleton -The worker binary also runs the asynq **scheduler**, which enqueues the eight +The worker binary also runs the asynq **scheduler**, which enqueues the nine periodic reconcile sweeps (enrollments, inbox, warmup, domain auth, recipient ESP, -maintenance cleanup, the fleet rotation pass, and the stranded manual-send sweep -that re-drives a scheduled reply or composed email whose task was lost). +maintenance cleanup, the [data retention](#data-retention) sweep, the fleet +rotation pass, and the stranded manual-send sweep that re-drives a scheduled +reply or composed email whose task was lost). asynq elects no leader. Every worker process with `INROAD_RUN_SCHEDULER=true` registers every periodic task independently, so **N replicas fire each sweep N @@ -401,7 +402,7 @@ level=INFO msg="scheduler disabled for this replica" run_scheduler=false ### Splitting control and send roles -By default a worker process runs everything: the scheduler, the eight periodic +By default a worker process runs everything: the scheduler, the nine periodic sweeps above, and every per-message handler (campaign sends, warmup ticks and engagement, inbox polls, manual replies, test sends, webhook deliveries). This is the self-host topology — one process, one trust domain, nothing to @@ -412,7 +413,7 @@ halves, **and which queues it consumes follows the same split** — that second half used to be missing, which is why this section used to warn you off using it. It doesn't any more; the topology below is operable. -- **`control`** — the scheduler and the eight periodic sweeps/purges. These scan +- **`control`** — the scheduler and the nine periodic sweeps/purges. These scan or delete across every workspace, so this role is meant to stay on trusted infrastructure beside the API. - **`send`** — per-message work only: campaign sends, warmup ticks and @@ -740,6 +741,206 @@ refusing it, or (rarely) when a destination beats the incumbent by a wide margin after 12 hours of residency. At most 20 moves per tick, 5 per destination. A fleet with one live worker never rotates at all. +## Data retention + +Five variables set how long the **retention sweep** (`maintenance:retention`, +hourly, on the one worker that runs the scheduler) keeps rows. Each is a whole +number of **days**. `0` or blank disables that table's sweep. `0` never means +"keep nothing". + +| Variable | What it removes | Default | Minimum | +| :--- | :--- | :--- | :--- | +| `INROAD_RETENTION_SENDS_DAYS` | Campaign sends (`sends`) that nothing live depends on, with their tracking events | **`0` (disabled)** | 90 | +| `INROAD_RETENTION_INBOX_DAYS` | Whole inbox conversations (`inbox_threads` + their `inbox_messages`) with no activity in the window | **`0` (disabled)** | 30 | +| `INROAD_RETENTION_TRACKING_EVENTS_DAYS` | Raw open/click events (`tracking_events`), after rolling them up | **`0` (disabled)** | 30 | +| `INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS` | Provider bounce/complaint events (`deliverability_events`) that no auto-pause rate still reads | **`0` (disabled)** | 90 | +| `INROAD_RETENTION_DEAD_LETTERS_DAYS` | Captured retry-exhausted tasks (`task_dead_letters`), every status | `90` | 7 | + +:::caution[The four recipient-data windows are off by default, and choosing them is not an engineering decision] +The first four windows cover five tables (`sends`, `inbox_threads`, +`inbox_messages`, `tracking_events`, `deliverability_events`). All five record +information about the **people your workspaces email**: addresses, message +bodies, when they opened what, and from which IP address. How long you may keep +that data, or must keep it, depends on your legal obligations, your contracts +and your own privacy policy. Inroad can't know any of that, so it keeps +everything until you set a window. **Ask whoever owns privacy and legal questions +for your deployment to pick these numbers.** This page describes what each +window does. It is not advice on what the windows should be. +::: + +**Malformed values stop the process.** The rules above for every other number +apply here too. On top of them, a negative number, a unit (`90d`, `720h`), a +fraction or anything over 36500 (a century) is refused at startup and the error +names the variable. A window **below its minimum** stops the *worker* at startup, +with the reason. Each minimum sits just above the widest window over which a rate +the product acts on reads that table, so a shorter window would change a +live decision, not just remove history. + +### What these windows do not cover + +These tables also hold data about recipients, and **no retention window +touches them**: + +| Table(s) | What it holds | +| :--- | :--- | +| `contacts`, `contact_emails` | The contact record itself | +| `suppression` | The unsubscribe and bounce list | +| `inbox_compose_drafts`, `inbox_pending_composes` | Emails a person wrote in the inbox composer and has not sent, or has queued | +| `inbox_pending_replies` | Queued manual replies (removed only with their conversation) | +| `crm_threads`, `crm_messages`, `crm_thread_participants`, `events` | The CRM's copy of conversations and its activity log | +| `notes`, `tasks` | Free text operators write about contacts | +| `agent_threads`, `agent_messages`, `agent_message_parts` | AI assistant chats, which may quote recipient data | +| `sequence_enrollments`, `list_members` | Who was enrolled in what | +| `webhook_deliveries` | Outbound webhook payloads (fixed 30-day purge) | +| `idempotency_keys` | Replayable API responses (fixed 24-hour purge) | + +Contacts are deleted by deleting the contact, which also removes their sends, +tracking and enrollments. Removing the suppression list would make an +unsubscribed person mailable again. **Whether any other table on this list needs +a window is also a Privacy/Legal decision.** This project doesn't make it. + +### What "delete" means for each table + +- **Tracking events are rolled up, not lost.** Before a raw event is deleted it is + folded into a per-send summary (`tracking_event_rollups`: send, open/click, + human/machine, a count, first and last time). All open and click numbers read + from the raw and rolled-up rows together. Campaign rates, per-step and + per-variant results, the campaign ranking, a contact's engagement and the bot + classifier's own inputs are therefore **unchanged** when events age out. What's + gone is the per-hit detail: user agent, client IP and click URL. That includes + the individual open/click entries in a CRM deal's activity feed. A future + re-run of bot classification over old events can't reach rolled-up rows, + because the evidence it would judge is exactly what the rollup dropped. +- **A deliverability event is kept while a rate can still read it.** Two kinds + are kept past the window: + - an event on a **running or paused** campaign's send, received after that + campaign's guardrails were switched on. The circuit breaker can fall back to + that whole period when recent volume is low, and a paused campaign is one + resume away from being assessed on it. + - each workspace's **newest complaint**. That row is what lets the dashboard + tell "complaint feed live, zero complaints" apart from "not measured". + + After deletion, a provider replaying the same event would be ingested again. The + 90-day minimum puts that far beyond any provider's retry window. +- **An inbox conversation is deleted whole**, messages and all, only when its last + activity is past the window. A conversation is kept if it has any of these: + - a reply still scheduled or sending + - a snooze that has not lapsed + - an active sequence enrollment + - any message inside the window +- **A send is deleted only when nothing live uses it.** A send is kept if it is: + - queued or sending (the row *is* the send claim) + - part of an **active** enrollment. The next step threads off it, and a reply is + matched back through it. + - on a **running or paused** campaign, sent after its guardrails were switched + on. A campaign that can still run keeps its sending history. + - shown in an inbox conversation that still exists (the campaign side of the + conversation is rebuilt from sends) + - named by a deliverability event that is still kept + - behind a CRM deal sourced from that campaign and contact + + Deleting a send removes it from **every** report (sent counts, per-step + results, the deliverability series), together with its tracking events and + rollups, so open and click rates stay consistent. Unsubscribe links in old mail + keep working because they don't depend on the send. What does stop working: + - **Tracked links and pixels in old mail.** A click on a link from a deleted + send returns 404 instead of redirecting, including a click that arrives while + its send is being deleted. + - **Matching late mail about it.** Inroad matches incoming mail to a campaign + through the send's Message-ID, so for a deleted send: + - a late **reply** isn't captured in the unified inbox, and doesn't stop or + mark any enrollment + - a late **bounce** doesn't suppress the address + - a mail-borne **abuse complaint** (ARF report) is declined rather than + ingested, so it neither counts toward a rate nor suppresses + + Each is logged (`no_matching_send`). Complaints and bounces posted by a + provider *webhook* are still recorded against the workspace; they just lose + their campaign attribution. + +The windows interact. A send stays while an inbox conversation or deliverability +event still holds it, so the sends window has no effect beyond those two unless +they are also enabled. Enabling sends retention while leaving tracking retention +disabled still removes a deleted send's tracking events, because they belong to +the send. + +### How the sweep runs + +- **One sweeper at a time.** Each run takes a Postgres advisory lock. A replica + whose run overlaps another does nothing and succeeds. This also keeps the + tracking rollup and the sends purge from running at once, which would otherwise + deadlock each other. +- **Bounded batches, resumed across runs.** A batch examines a fixed number of + rows past its cursor: 20,000 for sends, inbox conversations and deliverability + events, and 5,000 for tracking events and dead letters. It deletes the ones no + guard keeps. A row a guard keeps costs one scanned slot and is stepped over, + never re-read. Each table's position is saved in `retention_cursors`. The next + run resumes from there, and starts over from the oldest row when the table + drains, or after a day, so rows a guard stopped protecting are rechecked. +- **Each batch is capped at one minute** (a statement timeout inside its own + transaction). A run takes at most 40 batches per table and starts no new batch + after 5 minutes. A table that still has rows past its window logs + `retention backlog not drained this run` at WARN and continues next hour. +- **Ages use the database clock**, and rows the live path has locked (a send + being claimed) are skipped rather than waited on. +- **Observable.** Each run logs one `retention applied` line per enabled table, + with rows scanned, rows deleted, dependent rows and whether it drained. The + counter `inroad_retention_rows_deleted_total{table,scope}` counts deletions + (`scope="dependent"` is an inbox thread's messages, or a send's tracking rows). + The run is recorded as `retention` in the scheduled-job ledger. At startup the + worker logs which tables are enabled (`msg=retention enabled_days=…`). + +### Upgrading: the index migrations + +The release that ships retention adds these migrations: + +- `20260923110315`: the rollup table, the `tracking_engagement` view and the + cursor table. Instant, because all three are new and empty. +- `20260923144743` to `20260923144748`: one age or lookup index each, built + `CREATE INDEX CONCURRENTLY`. Writes to `sends`, `tracking_events` and the other + tables continue during the build. Each file is a single statement, which is + what lets golang-migrate run it outside a transaction; don't add a second. +- `20260923144749`: drops `idx_tracking_events_send` concurrently. A wider + existing index serves every lookup it served. +- `20260923144750`: autovacuum settings for `sends`, `tracking_events` and + `deliverability_events` (vacuum after 1% plus 50,000 dead rows, instead of + 20%). This does not block reads or writes. + +A concurrent build takes longer than a blocking one on a busy table. If one is +interrupted (a crash, a cancelled deploy), Postgres leaves the index behind marked +**INVALID** and golang-migrate marks the schema *dirty*. Because the file says +`IF NOT EXISTS`, simply re-running would skip the broken index. Recover like +this: + +```sql +-- find it +SELECT indexrelid::regclass FROM pg_index WHERE NOT indisvalid; +-- remove it +DROP INDEX CONCURRENTLY IF EXISTS ; +-- clear the dirty flag, back to the version before the failed file +UPDATE schema_migrations SET version = , dirty = false; +``` + +Then run the migrations again. + +### Enabling a window for the first time + +The first run after you enable a window meets the whole backlog at once. For a +deployment that has kept years of data: + +1. **Start wide and step down.** Enable, say, 730 days first. Once the WARN line + `retention backlog not drained` stops appearing for that table, lower the + window towards the target. Each step deletes a slice rather than everything. +2. **Watch the first days.** Watch `inroad_retention_rows_deleted_total`, the + WARN line and replication lag, if you have replicas. At the default caps a + table deletes at most about 4.8 million rows a day, so the backlog drains + over days rather than in one lock-heavy hour. +3. **Vacuum once it has drained.** Autovacuum is tuned to follow the sweep, but + after the first large drain run `VACUUM (ANALYZE)` on the tables you enabled. + This refreshes the planner's statistics. The disk space itself is reused + rather than returned to the OS; only `VACUUM FULL` or `pg_repack` shrinks the + files, and both need a maintenance window. + ## Database connection budget | Variable | Description | Default | diff --git a/docs/src/content/docs/security.md b/docs/src/content/docs/security.md index bc6ea7d8..91c891b4 100644 --- a/docs/src/content/docs/security.md +++ b/docs/src/content/docs/security.md @@ -2026,6 +2026,47 @@ write history that never happened. fleet network**, treat the token as equivalent to every mailbox credential in the installation, and rotate it on any suspicion. +82. **Recipient-data retention is off until an operator turns it on, and never + deletes what a live decision still reads.** (Number tentative; to be + renumbered on merge.) `maintenance:retention` + (`internal/worker/maintenance/retention.go`, SQL in `queries/retention.sql`) + sweeps `sends`, `inbox_threads`/`inbox_messages`, `tracking_events` and + `deliverability_events` only when their `INROAD_RETENTION_*_DAYS` is set. The + default is disabled because the window is a Privacy/Legal decision. Rules a + change must keep: + - **Disabled is zero calls.** A zero window is skipped before any query, and + the in-process client refuses a non-positive window anyway. Otherwise + `now() - 0` would match every row. + - **Floors are enforced at worker startup** (`RetentionPolicy.Validate`). A + window that would empty a table a rate still reads stops the process. + - **Guards are SQL, beside their reasons.** A send is never deleted while any + of these still uses it: an active enrollment, a running *or paused* + campaign's breaker window, an existing inbox thread, a retained + deliverability event, or a CRM deal. Two deliverability events are never + deleted: one inside a running or paused campaign's breaker window, and a + workspace's newest complaint (it keeps `complaint_feed` saying "measured"). + Paused counts as running because a resumed campaign is assessed on that + history at once. + - **Every reader of tracking events reads `tracking_engagement`** (raw plus + rolled up), never `tracking_events` directly. Otherwise it silently loses + everything older than the tracking window. The exceptions are an allowlist + with reasons, enforced by `internal/platform/db/trackingreaders_test.go`: + - the bot classifier's 10-minute burst rule + - the CRM deal activity feed, an event log that a rollup cannot preserve; + rolled-up events leave it + - the sandbox seeder + - retention itself + - **Tracking events roll up exactly once.** The rollup is inserted from the + DELETE's own `RETURNING`, in one statement. The rollup keeps no user agent, + client IP or URL. + - **Batches are bounded by rows scanned**, run in their own transaction with + a 60-second `statement_timeout`, and the run holds a single-sweeper advisory + lock. The lock is what keeps the rollup and the sends purge, which lock the + same rows in opposite orders, from running concurrently on two replicas. + - **Cross-tenant by design, control plane only.** Every batch deletes across + workspaces by age, so `maintenance.Retainer` is deliberately absent from + `*remote.Client` (invariant 73). The capability census asserts it is absent. + ## Fleet operator read surface 70. **The scheduled-job ledger never serves its error text.** `scheduled_job_runs` has no `workspace_id` and cannot honestly have one — a periodic reconcile runs diff --git a/internal/app/tracking/handler_test.go b/internal/app/tracking/handler_test.go index b4527ab1..f6385a13 100644 --- a/internal/app/tracking/handler_test.go +++ b/internal/app/tracking/handler_test.go @@ -41,11 +41,14 @@ type fakeStore struct { // subnets records the subnet PriorEvents was asked about, proving the // burst query is skipped when there is no usable client IP. subnets []netip.Prefix + // recordErr is what RecordEvent returns, to drive a send deleted between + // resolve and record (ErrSendGone) and an ordinary write failure. + recordErr error } func (f *fakeStore) RecordEvent(_ context.Context, ev Event) error { f.calls = append(f.calls, ev) - return nil + return f.recordErr } func (f *fakeStore) ResolveSend(_ context.Context, sendID uuid.UUID) (Send, bool) { @@ -268,6 +271,51 @@ func TestClickRedirect_UnknownSend_404NoRedirectNoEvent(t *testing.T) { } } +// A send deleted between ResolveSend and RecordEvent — the retention sweep +// winning the race against a click — must answer exactly as a send that was +// never found: 404, no redirect. Redirecting would make the outcome depend on +// which of two statements the delete landed between. +func TestClickRedirect_SendDeletedMidHit_404NoRedirect(t *testing.T) { + r, store, sendID := newTestHandler(t) + store.recordErr = ErrSendGone + tok := track.MakeClickToken(testSecret, sendID.String(), "https://example.test/landing") + + w := get(t, r, "/t/c/"+tok, testUA, "") + if w.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (the documented answer for a send that is gone)", w.Code) + } + if loc := w.Header().Get("Location"); loc != "" { + t.Fatalf("Location = %q, want no redirect", loc) + } +} + +// Any OTHER write failure is not a deleted send: the click still redirects, so a +// database blip never breaks a recipient's link. +func TestClickRedirect_RecordFailureStillRedirects(t *testing.T) { + r, store, sendID := newTestHandler(t) + store.recordErr = errors.New("connection reset") + dest := "https://example.test/landing" + tok := track.MakeClickToken(testSecret, sendID.String(), dest) + + w := get(t, r, "/t/c/"+tok, testUA, "") + if w.Code != http.StatusFound || w.Header().Get("Location") != dest { + t.Fatalf("status = %d Location = %q, want 302 to %s", w.Code, w.Header().Get("Location"), dest) + } +} + +// The pixel never fails, gone send or not: a mail client rendering it must see +// no difference an attacker could use to probe which sends exist. +func TestOpenGIF_SendDeletedMidHit_StillServesPixel(t *testing.T) { + r, store, sendID := newTestHandler(t) + store.recordErr = ErrSendGone + tok := track.MakeOpenToken(testSecret, sendID.String()) + + w := get(t, r, "/t/o/"+tok+".gif", testUA, "") + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (the same pixel as ever)", w.Code) + } +} + // --- Bot / prefetch classification --- // // The load-bearing assertion in every case below is that a machine event is diff --git a/internal/app/tracking/sendgone_integration_test.go b/internal/app/tracking/sendgone_integration_test.go new file mode 100644 index 00000000..a1a7e8db --- /dev/null +++ b/internal/app/tracking/sendgone_integration_test.go @@ -0,0 +1,39 @@ +//go:build integration + +package tracking + +import ( + "context" + "errors" + "testing" + + "github.com/inroad/inroad/internal/platform/botfilter" +) + +// The race the retention sweep makes possible: a hit resolves its send, the send +// is deleted, then the insert runs. Postgres refuses the insert on the tenant FK +// (23503), and the store must report that as ErrSendGone — the typed "this send +// is gone" the service answers with a 404 — not as a generic write failure that +// would be logged as an error and still redirect. +func TestRecordEventOnADeletedSendReportsErrSendGone(t *testing.T) { + ctx := context.Background() + pool, q, closePool := connect(t) + defer closePool() + fx := seedSend(t, ctx, pool, q) + store := NewPgStore(pool) + + send, ok := store.ResolveSend(ctx, fx.sendID) + if !ok { + t.Fatal("fixture send did not resolve") + } + if _, err := pool.Exec(ctx, `DELETE FROM sends WHERE id = $1`, fx.sendID); err != nil { + t.Fatalf("delete send: %v", err) + } + err := store.RecordEvent(ctx, Event{ + WorkspaceID: send.WorkspaceID, CampaignID: send.CampaignID, SendID: fx.sendID, + Kind: botfilter.KindClick, URL: "https://example.test/", UserAgent: "UA", + }) + if !errors.Is(err, ErrSendGone) { + t.Fatalf("RecordEvent on a deleted send = %v, want ErrSendGone", err) + } +} diff --git a/internal/app/tracking/service.go b/internal/app/tracking/service.go index 77286124..27b866e6 100644 --- a/internal/app/tracking/service.go +++ b/internal/app/tracking/service.go @@ -2,6 +2,7 @@ package tracking import ( "context" + "errors" "log/slog" "net/netip" "net/url" @@ -92,7 +93,12 @@ func (s *Service) RecordClick(ctx context.Context, hit Hit) (destURL string, ok if !ok { return "", false } - s.record(ctx, hit, sendID, send, botfilter.KindClick, rawURL) + // A send deleted between resolve and record answers exactly as a send that + // was never found does — 404, no redirect — so a click racing the retention + // sweep behaves like the click that arrives a second later. + if gone := s.record(ctx, hit, sendID, send, botfilter.KindClick, rawURL); gone { + return "", false + } return rawURL, true } @@ -115,8 +121,10 @@ func (s *Service) resolve(ctx context.Context, parse func() (string, bool)) (uui return sendID, send, true } -// record classifies the hit and stores it, machine verdicts included. -func (s *Service) record(ctx context.Context, hit Hit, sendID uuid.UUID, send Send, kind botfilter.Kind, rawURL string) { +// record classifies the hit and stores it, machine verdicts included. It reports +// whether the send turned out to be gone (ErrSendGone): the one store failure a +// caller answers differently, because it is not a failure but a deleted send. +func (s *Service) record(ctx context.Context, hit Hit, sendID uuid.UUID, send Send, kind botfilter.Kind, rawURL string) (sendGone bool) { verdict, reason := s.classify(ctx, hit, sendID, send, kind) err := s.store.RecordEvent(ctx, Event{ WorkspaceID: send.WorkspaceID, @@ -129,14 +137,18 @@ func (s *Service) record(ctx context.Context, hit Hit, sendID uuid.UUID, send Se Reason: reason, ClientIP: hit.IP, }) + if errors.Is(err, ErrSendGone) { + return true + } if err != nil { - // The response is already decided (a pixel, or a redirect), so there is - // nothing to return this to and nothing useful to tell the caller. It - // is logged rather than discarded so a broken tracking write is visible - // as something other than a mysteriously flat open rate. No token, no - // URL and no user agent: this endpoint's inputs are recipient data. + // Any other failure leaves the response as decided (a pixel, or a + // redirect): nothing is lost for the recipient. It is logged rather + // than discarded so a broken tracking write is visible as something + // other than a mysteriously flat open rate. No token, no URL and no + // user agent: this endpoint's inputs are recipient data. s.log.ErrorContext(ctx, "record tracking event", "error", err, "kind", string(kind), "send_id", sendID) } + return false } // classify gathers the classifier's inputs and returns its verdict. diff --git a/internal/app/tracking/store.go b/internal/app/tracking/store.go index ef2d883f..08c51fa6 100644 --- a/internal/app/tracking/store.go +++ b/internal/app/tracking/store.go @@ -12,11 +12,13 @@ package tracking import ( "context" + "errors" "fmt" "net/netip" "time" "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" @@ -43,14 +45,27 @@ type Event struct { ClientIP netip.Addr } +// ErrSendGone reports that the send a hit names was deleted between ResolveSend +// and RecordEvent — the retention sweep (maintenance:retention) deleting it, or +// its campaign or contact being deleted, in the moment between the two +// statements. It is the same outcome as ResolveSend finding nothing, reached one +// statement later, and callers treat it exactly like that: record nothing, and +// answer the way an unknown send is answered. It is not an error worth a log +// line; it is the documented behaviour of a link from a deleted send. +var ErrSendGone = errors.New("tracking: the send no longer exists") + +// foreignKeyViolation is Postgres's SQLSTATE for an insert whose parent row is +// gone — here, tracking_events' tenant FKs to sends and campaigns. +const foreignKeyViolation = "23503" + // Store is the repository interface this domain depends on. It is defined // here (by the consumer), not by the persistence layer, so the service can // be unit-tested against a fake without a database. type Store interface { // RecordEvent inserts a tracking_events row. WorkspaceID and CampaignID // must already be resolved server-side (via ResolveSend) -- callers - // must never pass values sourced from the token or the request, since - // send_id has no FK and is the only integrity boundary here. + // must never pass values sourced from the token or the request. A send + // deleted since it was resolved is reported as ErrSendGone. RecordEvent(ctx context.Context, ev Event) error // ResolveSend maps a sendID to the workspace/campaign that own it and the // time it was sent, looked up from the sends row itself. ok is false if no @@ -102,6 +117,10 @@ func (s *PgStore) RecordEvent(ctx context.Context, ev Event) error { params.ClientIp = &addr } if err := s.q.InsertTrackingEvent(ctx, params); err != nil { + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == foreignKeyViolation { + return ErrSendGone + } return fmt.Errorf("insert tracking event: %w", err) } return nil diff --git a/internal/coreapi/inprocess/jobrun.go b/internal/coreapi/inprocess/jobrun.go index ed9e2b80..b7de26c4 100644 --- a/internal/coreapi/inprocess/jobrun.go +++ b/internal/coreapi/inprocess/jobrun.go @@ -11,7 +11,7 @@ import ( // RecordJobRun implements jobrun.Recorder: persist one completed periodic // reconcile run. Called by internal/platform/jobrun.Record, wrapped around -// each of the eight sweeps in internal/worker/handlers.go — never by an HTTP +// each of the nine sweeps in internal/worker/handlers.go — never by an HTTP // caller, so there is no workspace to pin (scheduled_job_runs carries none; // see its migration's own doc for why these rows are instance-scoped). func (c client) RecordJobRun(ctx context.Context, run jobrun.Run) error { @@ -28,6 +28,6 @@ func (c client) RecordJobRun(ctx context.Context, run jobrun.Run) error { // The BINDING half of the type-assertion pattern, as in the five siblings that // carry one. internal/worker/handlers.go resolves this capability with // `core.(jobrun.Recorder)` and treats a miss as "record nothing, run the sweep -// anyway", so a signature drift here would make all eight periodic reconciles stop +// anyway", so a signature drift here would make all nine periodic reconciles stop // writing to the ledger with nothing failing anywhere. var _ jobrun.Recorder = client{} diff --git a/internal/coreapi/inprocess/maintenance.go b/internal/coreapi/inprocess/maintenance.go index 5b1d6076..6a67137b 100644 --- a/internal/coreapi/inprocess/maintenance.go +++ b/internal/coreapi/inprocess/maintenance.go @@ -27,17 +27,6 @@ func (c client) PurgeWarmupObservations(ctx context.Context) (int64, error) { return c.q.PurgeWarmupObservations(ctx) } -// PurgeDeadLetters removes captured retry-exhausted tasks past their 90-day -// retention window. task_dead_letters is append-only in practice — triage flips -// a status, it never deletes — and had no sweep at all, so it grew forever on a -// system whose failure mode is a provider outage failing hundreds of queued -// sends at once. Same reasoning as PurgeWarmupObservations above (invariant 55), -// and kept separate from CleanupExpired for the same reason: a dead letter is a -// record of dropped work, not an authentication artifact. -func (c client) PurgeDeadLetters(ctx context.Context) (int64, error) { - return c.q.PurgeTaskDeadLetters(ctx) -} - // PurgeWebhookDeliveries removes webhook_deliveries rows past their 30-day // retention window. task_dead_letters / warmup_observations reasoning // (invariant 55): the table grows one row per (event, endpoint) and is never @@ -61,7 +50,7 @@ func (c client) PurgeDeadWorkers(ctx context.Context) (int64, error) { // PurgeScheduledJobRuns removes scheduled_job_runs rows past their 30-day // retention window. task_dead_letters / webhook_deliveries reasoning -// (invariant 55): eight jobs write a row per run through +// (invariant 55): nine jobs write a row per run through // internal/platform/jobrun.Record, several every five minutes, and nothing // else in the codebase ever deletes from the table. Kept separate from // CleanupExpired for the same reason as the purges above: a job-run record is diff --git a/internal/coreapi/inprocess/maintenance_integration_test.go b/internal/coreapi/inprocess/maintenance_integration_test.go index 547c3fe2..b8b70a3e 100644 --- a/internal/coreapi/inprocess/maintenance_integration_test.go +++ b/internal/coreapi/inprocess/maintenance_integration_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/uuid" + "github.com/inroad/inroad/internal/coreapi" "github.com/inroad/inroad/internal/platform/jobrun" ) @@ -191,12 +192,15 @@ func TestPurgeDeadLettersPurgesOnlyOutsideRetentionWindow(t *testing.T) { expiredA, freshA := seed(wsA.ID, "91 days"), seed(wsA.ID, "89 days") expiredB, freshB := seed(wsB.ID, "91 days"), seed(wsB.ID, "89 days") - deleted, err := (client{q: q}).PurgeDeadLetters(ctx) + // The default window (config.DefaultRetentionDeadLettersDays), which the + // retention sweep now passes in rather than the query hard-coding. + req := coreapi.RetentionRequest{OlderThan: 90 * 24 * time.Hour, Limit: 5000} + res, err := (client{pool: pool, q: q}).PurgeDeadLetters(ctx, req) if err != nil { t.Fatalf("purge: %v", err) } - if deleted < 2 { - t.Fatalf("deleted rows = %d, want at least the 2 expired rows (one per workspace)", deleted) + if res.Deleted < 2 { + t.Fatalf("deleted rows = %d, want at least the 2 expired rows (one per workspace)", res.Deleted) } exists := func(id uuid.UUID) bool { @@ -224,7 +228,7 @@ func TestPurgeDeadLettersPurgesOnlyOutsideRetentionWindow(t *testing.T) { // Idempotent: the sweep runs daily, so "nothing left in window" is its steady // state rather than an edge case. - if _, err := (client{q: q}).PurgeDeadLetters(ctx); err != nil { + if _, err := (client{pool: pool, q: q}).PurgeDeadLetters(ctx, req); err != nil { t.Fatalf("second purge: %v", err) } if !exists(freshA) || !exists(freshB) { @@ -272,7 +276,7 @@ func TestRecordJobRunPersistsWhatTheDecoratorObserved(t *testing.T) { } } -// task_dead_letters / webhook_deliveries reasoning (invariant 55): eight jobs +// task_dead_letters / webhook_deliveries reasoning (invariant 55): nine jobs // write a row per run through jobrun.Record, several every five minutes, so // scheduled_job_runs needed a retention sweep from day one. Global and // unpinned like the other purges in this file — deployment maintenance, not diff --git a/internal/coreapi/inprocess/retention.go b/internal/coreapi/inprocess/retention.go new file mode 100644 index 00000000..69c12cc2 --- /dev/null +++ b/internal/coreapi/inprocess/retention.go @@ -0,0 +1,264 @@ +package inprocess + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "github.com/inroad/inroad/internal/coreapi" + "github.com/inroad/inroad/internal/platform/db/gen" +) + +// The recipient-data retention batches (maintenance.Retainer). Each method is one +// statement from queries/retention.sql or deadletter.sql, run in its own short +// transaction, and every policy decision (what to keep, why) lives in that SQL, +// beside the guards it states. What is here is the translation to and from the +// seam types, the per-batch statement budget, and the sweep's run-level lock and +// cursor — written once (retentionBatch) so the five batches cannot disagree on +// how a window or a cursor is encoded, or on refusing a non-positive window. +// +// None of these is on the remote transport; see coreapi/retention.go. + +// retentionStatementTimeout bounds ONE batch. The whole run is bounded by the +// worker (maintenance.RetentionOptions.Budget, 5 minutes) and killed by asynq at +// queue.sweepTimeout (10 minutes); this is the per-statement backstop underneath +// both, so a batch whose plan went wrong — or that queued behind a lock — fails +// on its own after a minute instead of holding its locks, and pinning the xmin +// horizon for every other table's vacuum, until the task is killed. A healthy +// batch is well under a second. +const retentionStatementTimeout = "60s" + +// retentionParams is the parameter shape every retention batch query shares. +// sqlc generates one params struct per query, all with these four fields in this +// order, so each converts to it directly (Go ignores struct tags in a +// conversion) — and a query whose parameters drifted would stop compiling here +// rather than binding the wrong value. +type retentionParams struct { + OlderThanSeconds int64 + AfterAt pgtype.Timestamptz + AfterID uuid.UUID + BatchLimit int32 +} + +// encodeRetention validates a request at the seam and encodes it. A window of +// zero or less is refused rather than passed through: `now() - 0s` is "every row +// in the table", and the only thing standing between a disabled policy and a +// full-table delete must not be every caller remembering to check. The sweep +// never sends one (a disabled table is skipped before any call); this is the +// backstop for the caller that would. +func encodeRetention(req coreapi.RetentionRequest) (retentionParams, error) { + if req.OlderThan <= 0 { + return retentionParams{}, fmt.Errorf("retention: window must be positive, got %s", req.OlderThan) + } + if req.Limit <= 0 { + return retentionParams{}, fmt.Errorf("retention: batch limit must be positive, got %d", req.Limit) + } + return retentionParams{ + // Truncated, not rounded: a fractional second can only make the cutoff a + // hair later, i.e. keep slightly MORE, which is the safe direction. + OlderThanSeconds: int64(req.OlderThan / time.Second), + // Valid even for the zero cursor: a NULL in the row comparison would + // match nothing, and the first batch would delete nothing. + AfterAt: pgtype.Timestamptz{Time: req.After.At, Valid: true}, + AfterID: req.After.ID, + BatchLimit: req.Limit, + }, nil +} + +func decodeCursor(at pgtype.Timestamptz, id uuid.UUID) coreapi.RetentionCursor { + return coreapi.RetentionCursor{At: at.Time, ID: id} +} + +// retentionBatch is the one shape all five share: validate and encode, run the +// statement in a transaction with SET LOCAL statement_timeout, translate its +// row. SET LOCAL needs the transaction — it would otherwise leak onto a pooled +// connection — and scopes the timeout to this batch alone. +func retentionBatch[R any](ctx context.Context, c client, req coreapi.RetentionRequest, what string, + query func(context.Context, *gen.Queries, retentionParams) (R, error), result func(R) coreapi.RetentionBatch) (batch coreapi.RetentionBatch, err error) { + p, err := encodeRetention(req) + if err != nil { + return coreapi.RetentionBatch{}, err + } + if c.pool == nil { + return coreapi.RetentionBatch{}, errors.New("retention: no database pool") + } + tx, err := c.pool.Begin(ctx) + if err != nil { + return coreapi.RetentionBatch{}, fmt.Errorf("%s: begin: %w", what, err) + } + defer func() { + if err != nil { + err = errors.Join(err, rollbackUnlessDone(ctx, tx)) + } + }() + if _, err := tx.Exec(ctx, "SET LOCAL statement_timeout = '"+retentionStatementTimeout+"'"); err != nil { + return coreapi.RetentionBatch{}, fmt.Errorf("%s: statement timeout: %w", what, err) + } + row, err := query(ctx, c.q.WithTx(tx), p) + if err != nil { + return coreapi.RetentionBatch{}, fmt.Errorf("%s: %w", what, err) + } + if err := tx.Commit(ctx); err != nil { + return coreapi.RetentionBatch{}, fmt.Errorf("%s: commit: %w", what, err) + } + return result(row), nil +} + +// rollbackUnlessDone rolls back a transaction that did not commit. A rollback +// after a successful commit reports pgx.ErrTxClosed, which is not a failure. +func rollbackUnlessDone(ctx context.Context, tx pgx.Tx) error { + // The rollback must happen even when ctx is what failed the batch: a + // cancelled context would otherwise leave the transaction open on a pooled + // connection until the pool noticed. + if err := tx.Rollback(context.WithoutCancel(ctx)); err != nil && !errors.Is(err, pgx.ErrTxClosed) { + return fmt.Errorf("rollback: %w", err) + } + return nil +} + +// RollupTrackingEvents folds one batch of raw tracking events past the window +// into tracking_event_rollups and deletes them, in one statement. There is no +// guard, so every row scanned is a row deleted. +func (c client) RollupTrackingEvents(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return retentionBatch(ctx, c, req, "roll up tracking events", + func(ctx context.Context, q *gen.Queries, p retentionParams) (gen.RollupTrackingEventsRow, error) { + return q.RollupTrackingEvents(ctx, gen.RollupTrackingEventsParams(p)) + }, + func(r gen.RollupTrackingEventsRow) coreapi.RetentionBatch { + return coreapi.RetentionBatch{Scanned: r.DeletedRows, Deleted: r.DeletedRows, RolledUp: r.RollupRows, Next: decodeCursor(r.LastAt, r.LastID)} + }) +} + +// PurgeDeliverabilityEvents deletes one batch of bounce/complaint events past the +// window that no rate still reads. +func (c client) PurgeDeliverabilityEvents(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return retentionBatch(ctx, c, req, "purge deliverability events", + func(ctx context.Context, q *gen.Queries, p retentionParams) (gen.PurgeDeliverabilityEventsRow, error) { + return q.PurgeDeliverabilityEvents(ctx, gen.PurgeDeliverabilityEventsParams(p)) + }, + func(r gen.PurgeDeliverabilityEventsRow) coreapi.RetentionBatch { + return coreapi.RetentionBatch{Scanned: r.ScannedRows, Deleted: r.DeletedRows, Next: decodeCursor(r.LastAt, r.LastID)} + }) +} + +// PurgeInboxThreads deletes one batch of whole inbox conversations with no +// activity inside the window, and their messages. +func (c client) PurgeInboxThreads(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return retentionBatch(ctx, c, req, "purge inbox threads", + func(ctx context.Context, q *gen.Queries, p retentionParams) (gen.PurgeInboxThreadsRow, error) { + return q.PurgeInboxThreads(ctx, gen.PurgeInboxThreadsParams(p)) + }, + func(r gen.PurgeInboxThreadsRow) coreapi.RetentionBatch { + return coreapi.RetentionBatch{Scanned: r.ScannedRows, Deleted: r.DeletedRows, Dependents: r.DeletedMessages, Next: decodeCursor(r.LastAt, r.LastID)} + }) +} + +// PurgeSends deletes one batch of campaign sends past the window that nothing +// live depends on, with their tracking events and rollups. +func (c client) PurgeSends(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return retentionBatch(ctx, c, req, "purge sends", + func(ctx context.Context, q *gen.Queries, p retentionParams) (gen.PurgeSendsRow, error) { + return q.PurgeSends(ctx, gen.PurgeSendsParams(p)) + }, + func(r gen.PurgeSendsRow) coreapi.RetentionBatch { + return coreapi.RetentionBatch{Scanned: r.ScannedRows, Deleted: r.DeletedRows, Dependents: r.DeletedTracking, Next: decodeCursor(r.LastAt, r.LastID)} + }) +} + +// PurgeDeadLetters deletes one batch of captured retry-exhausted tasks past the +// window. task_dead_letters is append-only in practice — triage flips a status, +// it never deletes — and had no sweep at all, so it grew forever on a system +// whose failure mode is a provider outage failing hundreds of queued sends at +// once (invariant 55's reasoning). It moved here from the daily maintenance +// cleanup, where its 90 days were hard-coded, so the window is the operator's +// (INROAD_RETENTION_DEAD_LETTERS_DAYS, default 90). No guard, so every row +// scanned is a row deleted. +func (c client) PurgeDeadLetters(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return retentionBatch(ctx, c, req, "purge dead letters", + func(ctx context.Context, q *gen.Queries, p retentionParams) (gen.PurgeTaskDeadLettersRow, error) { + return q.PurgeTaskDeadLetters(ctx, gen.PurgeTaskDeadLettersParams(p)) + }, + func(r gen.PurgeTaskDeadLettersRow) coreapi.RetentionBatch { + return coreapi.RetentionBatch{Scanned: r.DeletedRows, Deleted: r.DeletedRows, Next: decodeCursor(r.LastAt, r.LastID)} + }) +} + +// LoadRetentionProgress reads where a table's sweep got to. A table never swept +// has no row, which is the zero progress: start from the oldest row. +func (c client) LoadRetentionProgress(ctx context.Context, table string) (coreapi.RetentionProgress, error) { + row, err := c.q.GetRetentionCursor(ctx, table) + if errors.Is(err, pgx.ErrNoRows) { + return coreapi.RetentionProgress{}, nil + } + if err != nil { + return coreapi.RetentionProgress{}, fmt.Errorf("load retention cursor %s: %w", table, err) + } + return coreapi.RetentionProgress{Cursor: decodeCursor(row.AfterAt, row.AfterID), CycleExpired: row.CycleExpired}, nil +} + +// SaveRetentionProgress records a table's position. newCycle restarts the +// day-long cycle clock; the sweep sets it whenever it starts over from the +// oldest row. +func (c client) SaveRetentionProgress(ctx context.Context, table string, cursor coreapi.RetentionCursor, newCycle bool) error { + if err := c.q.SaveRetentionCursor(ctx, gen.SaveRetentionCursorParams{ + TableName: table, + AfterAt: pgtype.Timestamptz{Time: cursor.At, Valid: true}, + AfterID: cursor.ID, + NewCycle: newCycle, + }); err != nil { + return fmt.Errorf("save retention cursor %s: %w", table, err) + } + return nil +} + +// TryRetentionSweepLock takes the deployment-wide single-sweeper lock, or +// reports that another run holds it. The lock is session-level, so it lives on a +// connection taken out of the pool for the whole run and returned by release; if +// that connection dies the lock goes with it, which is the right failure — the +// next run takes it over rather than waiting on a lock nobody holds. +// +// release must be called exactly once when acquired is true. It unlocks on a +// context detached from the run's own, because the run's context is exactly +// what is cancelled when the worker shuts down mid-run, and the unlock is the +// one statement that must still happen then. If the unlock itself fails the +// connection is closed rather than returned to the pool: a pooled connection +// still holding the lock would make every later run report "another replica is +// sweeping" until it happened to be recycled. +func (c client) TryRetentionSweepLock(ctx context.Context) (release func() error, acquired bool, err error) { + if c.pool == nil { + return nil, false, errors.New("retention: no database pool") + } + conn, err := c.pool.Acquire(ctx) + if err != nil { + return nil, false, fmt.Errorf("retention lock: acquire connection: %w", err) + } + ok, err := gen.New(conn).TryRetentionSweepLock(ctx) + if err != nil || !ok { + conn.Release() + if err != nil { + return nil, false, fmt.Errorf("retention lock: %w", err) + } + return nil, false, nil + } + release = func() error { + unlockCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) + defer cancel() + released, uerr := gen.New(conn).ReleaseRetentionSweepLock(unlockCtx) + if uerr == nil && !released { + uerr = errors.New("the lock was not held by this session") + } + if uerr != nil { + closeErr := conn.Conn().Close(unlockCtx) + conn.Release() + return errors.Join(fmt.Errorf("retention lock: release: %w", uerr), closeErr) + } + conn.Release() + return nil + } + return release, true, nil +} diff --git a/internal/coreapi/inprocess/retention_integration_test.go b/internal/coreapi/inprocess/retention_integration_test.go new file mode 100644 index 00000000..62ad9eb0 --- /dev/null +++ b/internal/coreapi/inprocess/retention_integration_test.go @@ -0,0 +1,869 @@ +//go:build integration + +package inprocess + +import ( + "context" + "reflect" + "sort" + "testing" + "time" + + "github.com/google/uuid" + "github.com/hibiken/asynq" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/inroad/inroad/internal/coreapi" + "github.com/inroad/inroad/internal/platform/db/gen" + "github.com/inroad/inroad/internal/platform/queue" + "github.com/inroad/inroad/internal/worker/maintenance" +) + +// Retention batches against real Postgres. Every guard in queries/retention.sql +// is a NOT EXISTS over another table, and a fake store would agree with any of +// them, so this is where they are actually proven. +// +// The sweeps are GLOBAL and the test database is shared, so every assertion is +// about a row this test seeded, by id — never "the batch deleted N" — except in +// the batch-boundary tests, which seed rows thousands of days old so that no +// other fixture can be in their range. + +const retentionDay = 24 * time.Hour + +type retentionFixture struct { + pool *pgxpool.Pool + q *gen.Queries + c client + ws uuid.UUID + campaignID, mailboxID uuid.UUID + listID uuid.UUID +} + +func newRetentionFixture(t *testing.T) *retentionFixture { + t.Helper() + ctx := context.Background() + pool, q := claimConnect(t) + fx := seedForClaim(t, ctx, q) + var listID uuid.UUID + if err := pool.QueryRow(ctx, `SELECT list_id FROM campaigns WHERE id = $1`, fx.campaignID).Scan(&listID); err != nil { + t.Fatalf("campaign list: %v", err) + } + // Campaigns default to 'draft'; 'done' is the neutral state for these tests + // (not 'running', which is itself a guard). + if _, err := pool.Exec(ctx, `UPDATE campaigns SET status = 'done' WHERE id = $1`, fx.campaignID); err != nil { + t.Fatalf("campaign status: %v", err) + } + t.Cleanup(func() { + // Deals first: their contact and campaign FKs are RESTRICT, so the + // workspace cascade would otherwise refuse. Background context because + // the test's own has ended by the time cleanups run. + bg := context.Background() + if _, err := pool.Exec(bg, `DELETE FROM deals WHERE workspace_id = ANY($1)`, []uuid.UUID{fx.ws, fx.foreignWS}); err != nil { + t.Errorf("cleanup deals: %v", err) + } + if _, err := pool.Exec(bg, `DELETE FROM workspaces WHERE id = ANY($1)`, []uuid.UUID{fx.ws, fx.foreignWS}); err != nil { + t.Errorf("cleanup workspaces: %v", err) + } + }) + return &retentionFixture{pool: pool, q: q, c: client{pool: pool, q: q}, ws: fx.ws, campaignID: fx.campaignID, mailboxID: fx.mailboxID, listID: listID} +} + +func (f *retentionFixture) exec(t *testing.T, sql string, args ...any) { + t.Helper() + if _, err := f.pool.Exec(context.Background(), sql, args...); err != nil { + t.Fatalf("exec %q: %v", sql, err) + } +} + +func (f *retentionFixture) contact(t *testing.T) uuid.UUID { + t.Helper() + c, err := f.q.UpsertContact(context.Background(), gen.UpsertContactParams{ + WorkspaceID: f.ws, Email: "r-" + uuid.NewString() + "@x.test", FirstName: "R", + }) + if err != nil { + t.Fatalf("contact: %v", err) + } + return c.ID +} + +func (f *retentionFixture) campaign(t *testing.T) uuid.UUID { + t.Helper() + cam, err := f.q.CreateCampaign(context.Background(), gen.CreateCampaignParams{ + WorkspaceID: f.ws, Name: "Camp " + uuid.NewString(), MailboxID: f.mailboxID, ListID: f.listID, + Subject: "Hi", BodyText: "Hello", + }) + if err != nil { + t.Fatalf("campaign: %v", err) + } + f.exec(t, `UPDATE campaigns SET status = 'done' WHERE id = $1`, cam.ID) + return cam.ID +} + +// send seeds one sends row created ageDays ago; a 'sent' row is also sent then. +func (f *retentionFixture) send(t *testing.T, campaignID, contactID uuid.UUID, ageDays int, status string) uuid.UUID { + t.Helper() + var id uuid.UUID + if err := f.pool.QueryRow(context.Background(), ` + INSERT INTO sends (workspace_id, campaign_id, contact_id, mailbox_id, to_email, status, created_at, sent_at, step_order) + VALUES ($1, $2, $3, $4, 'r@x.test', $5, now() - make_interval(days => $6), + CASE WHEN $5 = 'sent' THEN now() - make_interval(days => $6) END, 1) + RETURNING id`, f.ws, campaignID, contactID, f.mailboxID, status, ageDays).Scan(&id); err != nil { + t.Fatalf("send: %v", err) + } + return id +} + +// event seeds one tracking event `age` ago. +func (f *retentionFixture) event(t *testing.T, sendID uuid.UUID, kind string, machine bool, age time.Duration) { + t.Helper() + reason := "" + if machine { + reason = "test_prefetch" + } + f.exec(t, ` + INSERT INTO tracking_events (workspace_id, campaign_id, send_id, kind, url, user_agent, is_machine, machine_reason, client_ip, created_at) + SELECT s.workspace_id, s.campaign_id, s.id, $2::tracking_event_kind, 'https://x.test/', 'UA', $3, $4, '203.0.113.9'::inet, + now() - make_interval(secs => $5) + FROM sends s WHERE s.id = $1`, sendID, kind, machine, reason, age.Seconds()) +} + +func (f *retentionFixture) enroll(t *testing.T, campaignID, contactID uuid.UUID, status string) { + t.Helper() + f.exec(t, `INSERT INTO sequence_enrollments (workspace_id, campaign_id, contact_id, status) VALUES ($1, $2, $3, $4)`, + f.ws, campaignID, contactID, status) +} + +// thread seeds an inbox thread idle for ageDays, with n messages at that age. +func (f *retentionFixture) thread(t *testing.T, campaignID, contactID *uuid.UUID, ageDays, n int) uuid.UUID { + t.Helper() + ctx := context.Background() + var id uuid.UUID + if err := f.pool.QueryRow(ctx, ` + INSERT INTO inbox_threads (workspace_id, mailbox_id, campaign_id, contact_id, root_message_id, subject, last_message_at, created_at) + VALUES ($1, $2, $3, $4, $5, 'Re: hi', now() - make_interval(days => $6), now() - make_interval(days => $6)) + RETURNING id`, f.ws, f.mailboxID, campaignID, contactID, "<"+uuid.NewString()+"@x.test>", ageDays).Scan(&id); err != nil { + t.Fatalf("thread: %v", err) + } + for range n { + f.message(t, id, ageDays) + } + return id +} + +func (f *retentionFixture) message(t *testing.T, threadID uuid.UUID, ageDays int) { + t.Helper() + f.exec(t, ` + INSERT INTO inbox_messages (thread_id, workspace_id, mailbox_id, direction, message_id, from_email, body_text, occurred_at) + VALUES ($1, $2, $3, 'inbound', $4, 'them@x.test', 'body', now() - make_interval(days => $5))`, + threadID, f.ws, f.mailboxID, "<"+uuid.NewString()+"@x.test>", ageDays) +} + +// deliverabilityEvent seeds one provider event ageDays old, optionally naming a send. +func (f *retentionFixture) deliverabilityEvent(t *testing.T, kind string, sendID *uuid.UUID, ageDays int) uuid.UUID { + t.Helper() + var id uuid.UUID + if err := f.pool.QueryRow(context.Background(), ` + INSERT INTO deliverability_events (workspace_id, kind, email, send_id, provider_event_id, bounce_class, received_at) + VALUES ($1, $2, 'r@x.test', $3, $4, CASE WHEN $2 = 'bounce' THEN 'hard' ELSE 'unknown' END, now() - make_interval(days => $5)) + RETURNING id`, f.ws, kind, sendID, "evt-"+uuid.NewString(), ageDays).Scan(&id); err != nil { + t.Fatalf("deliverability event: %v", err) + } + return id +} + +func (f *retentionFixture) exists(t *testing.T, table string, id uuid.UUID) bool { + t.Helper() + var ok bool + // table is always a literal from this file, never input. + if err := f.pool.QueryRow(context.Background(), `SELECT EXISTS(SELECT 1 FROM `+table+` WHERE id = $1)`, id).Scan(&ok); err != nil { + t.Fatalf("exists %s: %v", table, err) + } + return ok +} + +type batchFn func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) + +// drain runs batches the way the sweep does — resuming from each cursor — until +// a short batch, and returns the totals. +func drain(t *testing.T, fn batchFn, window time.Duration, limit int32) coreapi.RetentionBatch { + t.Helper() + var total coreapi.RetentionBatch + var cursor coreapi.RetentionCursor + for i := 0; i < 10_000; i++ { + res, err := fn(context.Background(), coreapi.RetentionRequest{OlderThan: window, Limit: limit, After: cursor}) + if err != nil { + t.Fatalf("batch %d: %v", i, err) + } + total.Deleted += res.Deleted + total.Dependents += res.Dependents + total.RolledUp += res.RolledUp + if res.Scanned < int64(limit) { + return total + } + cursor = res.Next + } + t.Fatal("retention never drained") + return total +} + +// --- tracking events: the rollup ------------------------------------------------ + +type trackingReports struct { + Engaged []gen.CountEngagedSendsByKindRow + ByVerdict []gen.CountTrackingEventsByKindAndVerdictRow + HumanOpens int64 + SendContext gen.GetSendTrackingContextRow + Contact gen.ContactTrackingStatsRow + Results []gen.CampaignSendResultsRow + Performance gen.ListCampaignPerformanceRow +} + +// reports reads every report that aggregates tracking events. +func (f *retentionFixture) reports(t *testing.T, campaignID, sendID, contactID uuid.UUID) trackingReports { + t.Helper() + ctx := context.Background() + var r trackingReports + var err error + if r.Engaged, err = f.q.CountEngagedSendsByKind(ctx, gen.CountEngagedSendsByKindParams{CampaignID: campaignID, WorkspaceID: f.ws}); err != nil { + t.Fatalf("engaged: %v", err) + } + sort.Slice(r.Engaged, func(i, j int) bool { return r.Engaged[i].Kind < r.Engaged[j].Kind }) + if r.ByVerdict, err = f.q.CountTrackingEventsByKindAndVerdict(ctx, gen.CountTrackingEventsByKindAndVerdictParams{CampaignID: campaignID, WorkspaceID: f.ws}); err != nil { + t.Fatalf("by verdict: %v", err) + } + sort.Slice(r.ByVerdict, func(i, j int) bool { + if r.ByVerdict[i].Kind != r.ByVerdict[j].Kind { + return r.ByVerdict[i].Kind < r.ByVerdict[j].Kind + } + return !r.ByVerdict[i].IsMachine && r.ByVerdict[j].IsMachine + }) + if r.HumanOpens, err = f.q.CountHumanOpens(ctx, gen.CountHumanOpensParams{CampaignID: campaignID, WorkspaceID: f.ws}); err != nil { + t.Fatalf("human opens: %v", err) + } + if r.SendContext, err = f.q.GetSendTrackingContext(ctx, sendID); err != nil { + t.Fatalf("send context: %v", err) + } + if r.Contact, err = f.q.ContactTrackingStats(ctx, gen.ContactTrackingStatsParams{WorkspaceID: f.ws, ContactID: contactID}); err != nil { + t.Fatalf("contact stats: %v", err) + } + if r.Results, err = f.q.CampaignSendResults(ctx, gen.CampaignSendResultsParams{CampaignID: campaignID, WorkspaceID: f.ws}); err != nil { + t.Fatalf("results: %v", err) + } + perf, err := f.q.ListCampaignPerformance(ctx, f.ws) + if err != nil { + t.Fatalf("performance: %v", err) + } + for _, p := range perf { + if p.ID == campaignID { + r.Performance = p + } + } + return r +} + +type rollupRow struct { + Kind string + IsMachine bool + Events int64 + FirstAt time.Time + LastAt time.Time +} + +func (f *retentionFixture) rollups(t *testing.T, sendID uuid.UUID) []rollupRow { + t.Helper() + rows, err := f.pool.Query(context.Background(), ` + SELECT kind::text, is_machine, events, first_at, last_at FROM tracking_event_rollups + WHERE send_id = $1 ORDER BY kind, is_machine`, sendID) + if err != nil { + t.Fatalf("rollups: %v", err) + } + defer rows.Close() + var out []rollupRow + for rows.Next() { + var r rollupRow + if err := rows.Scan(&r.Kind, &r.IsMachine, &r.Events, &r.FirstAt, &r.LastAt); err != nil { + t.Fatalf("scan rollup: %v", err) + } + out = append(out, r) + } + if err := rows.Err(); err != nil { + t.Fatalf("rollups: %v", err) + } + return out +} + +func (f *retentionFixture) rawEvents(t *testing.T, sendID uuid.UUID, olderThan time.Duration) int { + t.Helper() + var n int + if err := f.pool.QueryRow(context.Background(), ` + SELECT count(*) FROM tracking_events WHERE send_id = $1 AND created_at < now() - make_interval(secs => $2)`, + sendID, olderThan.Seconds()).Scan(&n); err != nil { + t.Fatalf("raw events: %v", err) + } + return n +} + +// THE rollup property: every report that reads tracking events says exactly the +// same thing before and after the raw rows past the window are rolled up — the +// campaign rates, the machine/human split, the per-variant results, the +// workspace ranking, the contact rollup, and the classifier's own per-send input. +// Meanwhile the raw rows (with their user agent, IP and URL) are gone. +func TestRollupTrackingEventsPreservesEveryReport(t *testing.T) { + f := newRetentionFixture(t) + window := 400 * retentionDay + contactA, contactB := f.contact(t), f.contact(t) + sendA := f.send(t, f.campaignID, contactA, 450, "sent") + sendB := f.send(t, f.campaignID, contactB, 450, "sent") + + for _, d := range []int{420, 421, 422} { + f.event(t, sendA, "open", false, time.Duration(d)*retentionDay) + } + f.event(t, sendA, "open", true, 423*retentionDay) + f.event(t, sendA, "open", true, 423*retentionDay) + f.event(t, sendA, "click", false, 424*retentionDay) + f.event(t, sendA, "open", false, retentionDay) // recent: stays raw + f.event(t, sendB, "click", true, 430*retentionDay) + f.event(t, sendB, "open", true, 431*retentionDay) + + before := f.reports(t, f.campaignID, sendA, contactA) + if before.SendContext.HumanOpens != 4 { + t.Fatalf("fixture: human opens of A = %d, want 4", before.SendContext.HumanOpens) + } + + res := drain(t, f.c.RollupTrackingEvents, window, 5000) + if res.Deleted < 8 || res.RolledUp < 5 { + t.Fatalf("rollup deleted %d raw rows into %d rollups, want at least this test's 8 into 5", res.Deleted, res.RolledUp) + } + + after := f.reports(t, f.campaignID, sendA, contactA) + if !reflect.DeepEqual(before, after) { + t.Fatalf("a report changed when its events were rolled up:\nbefore %+v\nafter %+v", before, after) + } + if n := f.rawEvents(t, sendA, window); n != 0 { + t.Errorf("%d raw events of A past the window survived", n) + } + if n := f.rawEvents(t, sendA, 0); n != 1 { + t.Errorf("raw events of A = %d, want the 1 recent one", n) + } + + got := f.rollups(t, sendA) + if len(got) != 3 { + t.Fatalf("rollups of A = %+v, want (click,human) (open,human) (open,machine)", got) + } + want := map[[2]any]int64{{"click", false}: 1, {"open", false}: 3, {"open", true}: 2} + for _, r := range got { + if want[[2]any{r.Kind, r.IsMachine}] != r.Events { + t.Errorf("rollup (%s, machine=%t) events = %d, want %d", r.Kind, r.IsMachine, r.Events, want[[2]any{r.Kind, r.IsMachine}]) + } + if r.Kind == "open" && !r.IsMachine { + if span := r.LastAt.Sub(r.FirstAt); span < 2*retentionDay-time.Minute || span > 2*retentionDay+time.Minute { + t.Errorf("human-open rollup spans %s, want the 2 days between its oldest and newest event", span) + } + } + } + + // Idempotent: a second pass over nothing new must not count anything twice. + drain(t, f.c.RollupTrackingEvents, window, 5000) + if again := f.reports(t, f.campaignID, sendA, contactA); !reflect.DeepEqual(before, again) { + t.Fatalf("a second rollup pass changed a report:\nbefore %+v\nafter %+v", before, again) + } + if again := f.rollups(t, sendA); !reflect.DeepEqual(got, again) { + t.Fatalf("a second rollup pass changed the rollups: %+v then %+v", got, again) + } +} + +// Rolling up a send whose rollup already exists must ADD to it and widen its +// time range — the ON CONFLICT path, which a single-pass test never reaches. +func TestRollupTrackingEventsMergesIntoAnExistingRollup(t *testing.T) { + f := newRetentionFixture(t) + s := f.send(t, f.campaignID, f.contact(t), 600, "sent") + for _, d := range []int{520, 510, 450, 440} { + f.event(t, s, "open", false, time.Duration(d)*retentionDay) + } + + drain(t, f.c.RollupTrackingEvents, 500*retentionDay, 5000) + first := f.rollups(t, s) + if len(first) != 1 || first[0].Events != 2 { + t.Fatalf("after the 500-day pass rollups = %+v, want one row of 2 events", first) + } + + drain(t, f.c.RollupTrackingEvents, 400*retentionDay, 5000) + merged := f.rollups(t, s) + if len(merged) != 1 || merged[0].Events != 4 { + t.Fatalf("after the 400-day pass rollups = %+v, want the same row grown to 4 events", merged) + } + if !merged[0].FirstAt.Equal(first[0].FirstAt) { + t.Errorf("first_at moved from %v to %v; LEAST must keep the oldest", first[0].FirstAt, merged[0].FirstAt) + } + if !merged[0].LastAt.After(first[0].LastAt.Add(50 * retentionDay)) { + t.Errorf("last_at = %v, want it advanced to the 440-day event (was %v)", merged[0].LastAt, first[0].LastAt) + } +} + +// Batch boundaries. The rows are ~8 years old so no other fixture in the shared +// database can fall in this window, which makes each batch's count exact: 7 rows +// at a limit of 3 is 3, 3, 1, and each batch resumes after the previous one's +// cursor rather than re-reading from the start. +func TestRollupTrackingEventsBatchesResumeFromTheCursor(t *testing.T) { + f := newRetentionFixture(t) + s := f.send(t, f.campaignID, f.contact(t), 3100, "sent") + for d := 3001; d <= 3007; d++ { + f.event(t, s, "open", false, time.Duration(d)*retentionDay) + } + window := 3000 * retentionDay + ctx := context.Background() + + var cursor coreapi.RetentionCursor + var counts []int64 + for range 4 { + res, err := f.c.RollupTrackingEvents(ctx, coreapi.RetentionRequest{OlderThan: window, Limit: 3, After: cursor}) + if err != nil { + t.Fatalf("batch: %v", err) + } + counts = append(counts, res.Deleted) + if res.Deleted < 3 { + break + } + if !res.Next.At.After(cursor.At) { + t.Fatalf("cursor did not advance: %v then %v", cursor, res.Next) + } + cursor = res.Next + } + if !reflect.DeepEqual(counts, []int64{3, 3, 1}) { + t.Fatalf("batch sizes = %v, want [3 3 1]", counts) + } + if got := f.rollups(t, s); len(got) != 1 || got[0].Events != 7 { + t.Fatalf("rollups = %+v, want one row of all 7 events", got) + } +} + +// --- deliverability events -------------------------------------------------- + +func TestPurgeDeliverabilityEventsRespectsTheWindowAndEveryRateGuard(t *testing.T) { + f := newRetentionFixture(t) + window := 400 * retentionDay + ctx := context.Background() + + oldBounce := f.deliverabilityEvent(t, "bounce", nil, 401) + freshBounce := f.deliverabilityEvent(t, "bounce", nil, 399) + // Two complaints past the window: only the older may go. The newer is this + // workspace's only evidence that a complaint feed exists. + olderComplaint := f.deliverabilityEvent(t, "complaint", nil, 402) + newestComplaint := f.deliverabilityEvent(t, "complaint", nil, 401) + + drain(t, f.c.PurgeDeliverabilityEvents, window, 5000) + for name, tc := range map[string]struct { + id uuid.UUID + want bool + }{ + "bounce past the window": {oldBounce, false}, + "bounce inside the window": {freshBounce, true}, + "older complaint past the window": {olderComplaint, false}, + "the workspace's newest complaint": {newestComplaint, true}, + } { + if got := f.exists(t, "deliverability_events", tc.id); got != tc.want { + t.Errorf("%s: exists = %t, want %t", name, got, tc.want) + } + } + counts, err := f.q.GetWorkspaceDeliverabilityCounts(ctx, gen.GetWorkspaceDeliverabilityCountsParams{ + WorkspaceID: f.ws, Since: pgtype.Timestamptz{Time: time.Now().Add(-7 * retentionDay), Valid: true}, + }) + if err != nil { + t.Fatalf("counts: %v", err) + } + if !counts.ComplaintFeed { + t.Error("complaint_feed flipped to false: retention made a measured workspace read as NOT MEASURED") + } + + // A running campaign's breaker can fall back to everything since + // guardrails_enabled_at, however old. An event inside that is kept... + campaign := f.campaign(t) + s := f.send(t, campaign, f.contact(t), 405, "sent") + guarded := f.deliverabilityEvent(t, "bounce", &s, 401) + f.exec(t, `UPDATE campaigns SET status = 'running', guardrails_enabled_at = now() - interval '500 days' WHERE id = $1`, campaign) + drain(t, f.c.PurgeDeliverabilityEvents, window, 5000) + if !f.exists(t, "deliverability_events", guarded) { + t.Fatal("an event inside a running campaign's breaker window was deleted") + } + // ...and one from before supervision began is not, even while running. + f.exec(t, `UPDATE campaigns SET guardrails_enabled_at = now() - interval '300 days' WHERE id = $1`, campaign) + drain(t, f.c.PurgeDeliverabilityEvents, window, 5000) + if f.exists(t, "deliverability_events", guarded) { + t.Fatal("an event from before the campaign's guardrails were enabled was kept") + } +} + +// --- inbox threads ---------------------------------------------------------- + +func TestPurgeInboxThreadsDeletesWholeIdleConversationsAndKeepsGuardedOnes(t *testing.T) { + f := newRetentionFixture(t) + window := 400 * retentionDay + + idle := f.thread(t, nil, nil, 401, 3) + recent := f.thread(t, nil, nil, 10, 1) + + scheduledReply := f.thread(t, nil, nil, 401, 1) + f.exec(t, `INSERT INTO inbox_pending_replies (workspace_id, thread_id, body_text, status, send_after) + VALUES ($1, $2, 'queued words', 'scheduled', now() + interval '1 hour')`, f.ws, scheduledReply) + finishedReply := f.thread(t, nil, nil, 401, 1) + f.exec(t, `INSERT INTO inbox_pending_replies (workspace_id, thread_id, body_text, status, send_after, sent_at) + VALUES ($1, $2, 'sent words', 'sent', now() - interval '401 days', now() - interval '401 days')`, f.ws, finishedReply) + + snoozedAhead := f.thread(t, nil, nil, 401, 1) + f.exec(t, `INSERT INTO inbox_thread_snoozes (thread_id, workspace_id, snooze_until) VALUES ($1, $2, now() + interval '1 day')`, snoozedAhead, f.ws) + snoozeLapsed := f.thread(t, nil, nil, 401, 1) + f.exec(t, `INSERT INTO inbox_thread_snoozes (thread_id, workspace_id, snooze_until) VALUES ($1, $2, now() - interval '1 day')`, snoozeLapsed, f.ws) + + activeContact, doneContact := f.contact(t), f.contact(t) + f.enroll(t, f.campaignID, activeContact, "active") + f.enroll(t, f.campaignID, doneContact, "completed") + activeSequence := f.thread(t, &f.campaignID, &activeContact, 401, 1) + finishedSequence := f.thread(t, &f.campaignID, &doneContact, 401, 1) + + recentMessage := f.thread(t, nil, nil, 401, 1) + f.message(t, recentMessage, 5) // a writer that forgot to bump last_message_at + + res := drain(t, f.c.PurgeInboxThreads, window, 5000) + if res.Dependents < 3+1+1+1 { + t.Errorf("messages deleted = %d, want at least the idle threads' 6", res.Dependents) + } + for name, tc := range map[string]struct { + id uuid.UUID + want bool + }{ + "idle thread": {idle, false}, + "recent thread": {recent, true}, + "thread with a reply still scheduled": {scheduledReply, true}, + "thread whose reply already went": {finishedReply, false}, + "thread snoozed into the future": {snoozedAhead, true}, + "thread whose snooze lapsed": {snoozeLapsed, false}, + "thread of an active enrollment": {activeSequence, true}, + "thread of a completed enrollment": {finishedSequence, false}, + "thread with a recent message": {recentMessage, true}, + } { + if got := f.exists(t, "inbox_threads", tc.id); got != tc.want { + t.Errorf("%s: exists = %t, want %t", name, got, tc.want) + } + } + var orphans int + if err := f.pool.QueryRow(context.Background(), `SELECT count(*) FROM inbox_messages WHERE thread_id = ANY($1)`, + []uuid.UUID{idle, finishedReply, snoozeLapsed, finishedSequence}).Scan(&orphans); err != nil { + t.Fatalf("orphans: %v", err) + } + if orphans != 0 { + t.Errorf("%d messages of deleted threads survived", orphans) + } +} + +// --- sends ------------------------------------------------------------------ + +func TestPurgeSendsDeletesOnlySendsNothingLiveDependsOn(t *testing.T) { + f := newRetentionFixture(t) + window := 400 * retentionDay + ctx := context.Background() + + // Free: a completed enrollment, no thread, no event, no deal. Its tracking + // events and rollup go with it. + freeContact := f.contact(t) + f.enroll(t, f.campaignID, freeContact, "completed") + free := f.send(t, f.campaignID, freeContact, 401, "sent") + f.event(t, free, "open", false, 401*retentionDay) + f.event(t, free, "click", false, 401*retentionDay) + f.exec(t, `INSERT INTO tracking_event_rollups (workspace_id, campaign_id, send_id, kind, is_machine, events, first_at, last_at) + VALUES ($1, $2, $3, 'open', true, 5, now() - interval '500 days', now() - interval '450 days')`, f.ws, f.campaignID, free) + failed := f.send(t, f.campaignID, f.contact(t), 401, "failed") + + recent := f.send(t, f.campaignID, f.contact(t), 399, "sent") + queued := f.send(t, f.campaignID, f.contact(t), 401, "queued") + + activeContact := f.contact(t) + f.enroll(t, f.campaignID, activeContact, "active") + active := f.send(t, f.campaignID, activeContact, 401, "sent") + + threadContact := f.contact(t) + threaded := f.send(t, f.campaignID, threadContact, 401, "sent") + f.thread(t, &f.campaignID, &threadContact, 10, 1) + + evented := f.send(t, f.campaignID, f.contact(t), 401, "sent") + f.deliverabilityEvent(t, "bounce", &evented, 10) + + dealContact := f.contact(t) + dealt := f.send(t, f.campaignID, dealContact, 401, "sent") + var pipelineID, stageID uuid.UUID + if err := f.pool.QueryRow(ctx, `INSERT INTO pipelines (workspace_id, name) VALUES ($1, 'P') RETURNING id`, f.ws).Scan(&pipelineID); err != nil { + t.Fatalf("pipeline: %v", err) + } + if err := f.pool.QueryRow(ctx, `INSERT INTO pipeline_stages (pipeline_id, workspace_id, key, label, color, position) + VALUES ($1, $2, 'new', 'New', '#888888', 1) RETURNING id`, pipelineID, f.ws).Scan(&stageID); err != nil { + t.Fatalf("stage: %v", err) + } + f.exec(t, `INSERT INTO deals (workspace_id, pipeline_id, stage_id, name, primary_contact_id, source_campaign_id) + VALUES ($1, $2, $3, 'Deal', $4, $5)`, f.ws, pipelineID, stageID, dealContact, f.campaignID) + + res := drain(t, f.c.PurgeSends, window, 5000) + if res.Dependents < 3 { + t.Errorf("dependent rows deleted = %d, want at least the free send's 2 events + 1 rollup", res.Dependents) + } + for name, tc := range map[string]struct { + id uuid.UUID + want bool + }{ + "sent, nothing depends on it": {free, false}, + "failed, never sent": {failed, false}, + "inside the window": {recent, true}, + "still queued (the row is the claim)": {queued, true}, + "its enrollment is active": {active, true}, + "an inbox thread renders it": {threaded, true}, + "a deliverability event names it": {evented, true}, + "a deal is sourced from its conversation": {dealt, true}, + } { + if got := f.exists(t, "sends", tc.id); got != tc.want { + t.Errorf("%s: exists = %t, want %t", name, got, tc.want) + } + } + var leftovers int + if err := f.pool.QueryRow(ctx, `SELECT (SELECT count(*) FROM tracking_events WHERE send_id = $1) + + (SELECT count(*) FROM tracking_event_rollups WHERE send_id = $1)`, free).Scan(&leftovers); err != nil { + t.Fatalf("leftovers: %v", err) + } + if leftovers != 0 { + t.Errorf("%d tracking rows of a deleted send survived", leftovers) + } + + // The running-campaign guard, on its own campaign so the status flip touches + // nothing above. + running := f.campaign(t) + runningSend := f.send(t, running, f.contact(t), 401, "sent") + f.exec(t, `UPDATE campaigns SET status = 'running', guardrails_enabled_at = now() - interval '500 days' WHERE id = $1`, running) + drain(t, f.c.PurgeSends, window, 5000) + if !f.exists(t, "sends", runningSend) { + t.Fatal("a send inside a running campaign's breaker window was deleted") + } + f.exec(t, `UPDATE campaigns SET guardrails_enabled_at = now() - interval '300 days' WHERE id = $1`, running) + drain(t, f.c.PurgeSends, window, 5000) + if f.exists(t, "sends", runningSend) { + t.Fatal("a running campaign's send from before its guardrails were enabled was kept") + } +} + +// Batches are bounded by rows SCANNED, and the cursor moves past a kept row +// rather than stopping at it: ~8-year-old rows so the counts are exact. Five +// rows, the second one guarded, limit 2: batch one scans rows 0-1 and deletes +// only row 0, batch two scans and deletes 2-3, batch three scans the last row +// and is short — drained. A kept row costs one scanned slot, never a stall, and +// the batch after it resumes BEHIND it instead of re-reading it. +func TestPurgeSendsKeysetWalksPastGuardedRows(t *testing.T) { + f := newRetentionFixture(t) + window := 3000 * retentionDay + var ids []uuid.UUID + for i, age := range []int{3010, 3009, 3008, 3007, 3006} { + c := f.contact(t) + if i == 1 { + f.enroll(t, f.campaignID, c, "active") + } + ids = append(ids, f.send(t, f.campaignID, c, age, "sent")) + } + + ctx := context.Background() + var cursor coreapi.RetentionCursor + var scanned, deleted []int64 + for range 5 { + res, err := f.c.PurgeSends(ctx, coreapi.RetentionRequest{OlderThan: window, Limit: 2, After: cursor}) + if err != nil { + t.Fatalf("batch: %v", err) + } + scanned, deleted = append(scanned, res.Scanned), append(deleted, res.Deleted) + if res.Scanned < 2 { + break + } + cursor = res.Next + } + if !reflect.DeepEqual(scanned, []int64{2, 2, 1}) || !reflect.DeepEqual(deleted, []int64{1, 2, 1}) { + t.Fatalf("batches scanned %v deleted %v, want scanned [2 2 1] deleted [1 2 1]", scanned, deleted) + } + for i, id := range ids { + if got, want := f.exists(t, "sends", id), i == 1; got != want { + t.Errorf("send %d exists = %t, want %t", i, got, want) + } + } +} + +// --- the sweep end to end --------------------------------------------------- + +// Disabled is a no-op against the real database, and enabled runs the tables in +// an order that frees what it can in ONE run: a send held by an old +// deliverability event and an old thread is released by those two tables' +// sweeps and deleted by the sends sweep a moment later. +func TestRetentionHandlerDisabledIsANoOpAndEnabledChainsTables(t *testing.T) { + f := newRetentionFixture(t) + contact := f.contact(t) + f.enroll(t, f.campaignID, contact, "completed") + s := f.send(t, f.campaignID, contact, 410, "sent") + f.event(t, s, "open", false, 410*retentionDay) + ev := f.deliverabilityEvent(t, "bounce", &s, 410) + th := f.thread(t, &f.campaignID, &contact, 410, 2) + dl := uuid.New() + f.exec(t, `INSERT INTO task_dead_letters (id, workspace_id, task_type, payload, created_at) + VALUES ($1, $2, 'sequence:advance', '{}'::jsonb, now() - interval '410 days')`, dl, f.ws) + + run := func(p maintenance.RetentionPolicy) { + t.Helper() + h := maintenance.RetentionHandler(f.c, p, nil, maintenance.RetentionOptions{}) + if err := h(context.Background(), asynq.NewTask(queue.TaskMaintenanceRetention, nil)); err != nil { + t.Fatalf("handler: %v", err) + } + } + + run(maintenance.RetentionPolicy{}) + for table, id := range map[string]uuid.UUID{"sends": s, "deliverability_events": ev, "inbox_threads": th, "task_dead_letters": dl} { + if !f.exists(t, table, id) { + t.Errorf("a DISABLED policy deleted a %s row", table) + } + } + if n := f.rawEvents(t, s, 0); n != 1 { + t.Errorf("a DISABLED policy touched tracking events (%d raw left, want 1)", n) + } + + w := 400 * retentionDay + run(maintenance.RetentionPolicy{DeliverabilityEvents: w, InboxThreads: w, TrackingEvents: w, Sends: w, DeadLetters: w}) + for table, id := range map[string]uuid.UUID{"sends": s, "deliverability_events": ev, "inbox_threads": th, "task_dead_letters": dl} { + if f.exists(t, table, id) { + t.Errorf("an enabled policy left the %s row in place", table) + } + } +} + +// --- review follow-ups -------------------------------------------------------- + +// A PAUSED campaign keeps its breaker history exactly as a running one does. Its +// stopped-as-bounced enrollments survive every sweep, so if its sends and events +// were pruned while it was paused, the moment it resumed the breaker would divide +// a surviving bounce numerator by a shrunken delivered denominator and pause it +// again. The assertion is on the breaker's own inputs +// (GetCampaignDeliverabilityCounts over the fallback window): identical before +// and after a purge while paused, and so after the resume. +func TestPausedCampaignKeepsItsBreakerHistoryThroughRetention(t *testing.T) { + f := newRetentionFixture(t) + window := 400 * retentionDay + ctx := context.Background() + campaign := f.campaign(t) + f.exec(t, `UPDATE campaigns SET status = 'paused', guardrails_enabled_at = now() - interval '500 days' WHERE id = $1`, campaign) + + var sends []uuid.UUID + for range 20 { + sends = append(sends, f.send(t, campaign, f.contact(t), 410, "sent")) + } + for _, s := range sends[:3] { + f.deliverabilityEvent(t, "bounce", &s, 405) + } + since := pgtype.Timestamptz{Time: time.Now().Add(-500 * retentionDay), Valid: true} + counts := func() gen.GetCampaignDeliverabilityCountsRow { + t.Helper() + row, err := f.q.GetCampaignDeliverabilityCounts(ctx, gen.GetCampaignDeliverabilityCountsParams{ + WorkspaceID: f.ws, CampaignID: campaign, Since: since, + }) + if err != nil { + t.Fatalf("counts: %v", err) + } + return row + } + before := counts() + if before.Delivered != 20 || before.Bounced != 3 { + t.Fatalf("fixture: delivered=%d bounced=%d, want 20/3", before.Delivered, before.Bounced) + } + + drain(t, f.c.PurgeDeliverabilityEvents, window, 5000) + drain(t, f.c.PurgeSends, window, 5000) + f.exec(t, `UPDATE campaigns SET status = 'running' WHERE id = $1`, campaign) + if after := counts(); after != before { + t.Fatalf("breaker inputs changed across a purge while paused: before %+v, after resume %+v", before, after) + } +} + +// The single-sweeper lock is exclusive across sessions and released cleanly, so +// a second replica's run is refused while the first holds it and succeeds after. +func TestRetentionSweepLockIsExclusiveAndReleases(t *testing.T) { + f := newRetentionFixture(t) + ctx := context.Background() + release, ok, err := f.c.TryRetentionSweepLock(ctx) + if err != nil || !ok { + t.Fatalf("first lock: ok=%t err=%v", ok, err) + } + if _, ok2, err := f.c.TryRetentionSweepLock(ctx); err != nil || ok2 { + t.Fatalf("second lock while held: ok=%t err=%v, want refused", ok2, err) + } + if err := release(); err != nil { + t.Fatalf("release: %v", err) + } + again, ok3, err := f.c.TryRetentionSweepLock(ctx) + if err != nil || !ok3 { + t.Fatalf("lock after release: ok=%t err=%v", ok3, err) + } + if err := again(); err != nil { + t.Fatalf("second release: %v", err) + } +} + +// Progress round-trips, and the day-long cycle is judged on the database clock: +// a cycle started more than 24 hours ago reports expired, and saving with +// newCycle restarts it. +func TestRetentionProgressRoundTripsAndExpiresDaily(t *testing.T) { + f := newRetentionFixture(t) + ctx := context.Background() + table := "test-table-" + uuid.NewString() + t.Cleanup(func() { + if _, err := f.pool.Exec(context.Background(), `DELETE FROM retention_cursors WHERE table_name = $1`, table); err != nil { + t.Errorf("cleanup cursor: %v", err) + } + }) + + if got, err := f.c.LoadRetentionProgress(ctx, table); err != nil || got != (coreapi.RetentionProgress{}) { + t.Fatalf("never-swept table = %+v, %v; want the zero progress", got, err) + } + want := coreapi.RetentionCursor{At: time.Date(2025, 1, 2, 3, 4, 5, 0, time.UTC), ID: uuid.New()} + if err := f.c.SaveRetentionProgress(ctx, table, want, false); err != nil { + t.Fatalf("save: %v", err) + } + got, err := f.c.LoadRetentionProgress(ctx, table) + if err != nil || !got.Cursor.At.Equal(want.At) || got.Cursor.ID != want.ID || got.CycleExpired { + t.Fatalf("loaded %+v, %v; want %+v, not expired", got, err, want) + } + + f.exec(t, `UPDATE retention_cursors SET cycle_started_at = now() - interval '25 hours' WHERE table_name = $1`, table) + if got, err := f.c.LoadRetentionProgress(ctx, table); err != nil || !got.CycleExpired { + t.Fatalf("a 25-hour-old cycle = %+v, %v; want expired", got, err) + } + if err := f.c.SaveRetentionProgress(ctx, table, coreapi.RetentionCursor{}, true); err != nil { + t.Fatalf("restart: %v", err) + } + if got, err := f.c.LoadRetentionProgress(ctx, table); err != nil || got.CycleExpired { + t.Fatalf("after a new cycle = %+v, %v; want not expired", got, err) + } +} + +// SET LOCAL scopes the batch's statement timeout to its own transaction: the +// pooled connection it ran on must come back with the server default, or every +// later query on it would inherit a one-minute ceiling. +func TestRetentionStatementTimeoutDoesNotLeakOntoThePool(t *testing.T) { + f := newRetentionFixture(t) + ctx := context.Background() + var before string + if err := f.pool.QueryRow(ctx, `SHOW statement_timeout`).Scan(&before); err != nil { + t.Fatalf("show: %v", err) + } + for range 3 { + if _, err := f.c.PurgeDeadLetters(ctx, coreapi.RetentionRequest{OlderThan: 3000 * retentionDay, Limit: 1}); err != nil { + t.Fatalf("batch: %v", err) + } + } + for range 8 { // more than the test pool's connections, so every one is checked + var after string + if err := f.pool.QueryRow(ctx, `SHOW statement_timeout`).Scan(&after); err != nil { + t.Fatalf("show: %v", err) + } + if after != before { + t.Fatalf("statement_timeout on a pooled connection = %q after a batch, want the default %q", after, before) + } + } +} diff --git a/internal/coreapi/inprocess/retention_test.go b/internal/coreapi/inprocess/retention_test.go new file mode 100644 index 00000000..c3deae99 --- /dev/null +++ b/internal/coreapi/inprocess/retention_test.go @@ -0,0 +1,50 @@ +package inprocess + +import ( + "context" + "testing" + "time" + + "github.com/inroad/inroad/internal/coreapi" +) + +// A zero window is `now() - 0s`: every row in the table. The sweep skips a +// disabled table before calling, but the seam must refuse one on its own, so a +// future caller that forgets cannot turn "disabled" into "delete everything". +// These run with a nil query set: the refusal must happen BEFORE any SQL, and a +// nil dereference here would mean it did not. +func TestRetentionBatchesRefuseANonPositiveWindowBeforeAnySQL(t *testing.T) { + c := client{} + methods := map[string]func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error){ + "RollupTrackingEvents": c.RollupTrackingEvents, + "PurgeDeliverabilityEvents": c.PurgeDeliverabilityEvents, + "PurgeInboxThreads": c.PurgeInboxThreads, + "PurgeSends": c.PurgeSends, + "PurgeDeadLetters": c.PurgeDeadLetters, + } + for name, fn := range methods { + for _, req := range []coreapi.RetentionRequest{ + {OlderThan: 0, Limit: 10}, + {OlderThan: -time.Hour, Limit: 10}, + {OlderThan: time.Hour, Limit: 0}, + } { + if _, err := fn(context.Background(), req); err == nil { + t.Errorf("%s(%+v) = nil error, want a refusal", name, req) + } + } + } +} + +func TestEncodeRetentionTruncatesToWholeSeconds(t *testing.T) { + args, err := encodeRetention(coreapi.RetentionRequest{OlderThan: 90*24*time.Hour + 1500*time.Millisecond, Limit: 1}) + if err != nil { + t.Fatalf("encode: %v", err) + } + // Truncating keeps the cutoff a hair LATER, i.e. keeps slightly more. + if want := int64(90*24*3600 + 1); args.OlderThanSeconds != want { + t.Fatalf("seconds = %d, want %d", args.OlderThanSeconds, want) + } + if !args.AfterAt.Valid { + t.Fatal("the zero cursor must still be sent as a real timestamp, not NULL (a NULL row comparison matches nothing)") + } +} diff --git a/internal/coreapi/remote/controlplane.go b/internal/coreapi/remote/controlplane.go index 4535da17..ee8646b7 100644 --- a/internal/coreapi/remote/controlplane.go +++ b/internal/coreapi/remote/controlplane.go @@ -49,7 +49,7 @@ import ( // the method and says why is the only one of the three an operator can act on. // // The same reasoning does NOT extend to the optional capability interfaces -// (maintenance.Cleaner, deliverability.Breaker, recipientesp.Core, +// (maintenance.Cleaner, maintenance.Retainer, deliverability.Breaker, recipientesp.Core, // fleet.Rotator, jobrun.Recorder). Those are consumed through comma-ok type // assertions, so NOT implementing them is already an explicit, handled answer — // the registrar logs and skips. Implementing them here to return an error would diff --git a/internal/coreapi/retention.go b/internal/coreapi/retention.go new file mode 100644 index 00000000..fb6f6ccd --- /dev/null +++ b/internal/coreapi/retention.go @@ -0,0 +1,76 @@ +package coreapi + +import ( + "time" + + "github.com/google/uuid" +) + +// Seam types for the recipient-data retention sweep (maintenance:retention). +// +// The methods that carry them are deliberately NOT on Client, for the reason +// stated on BreakerResult: the sweep depends on a narrow interface it defines +// itself (maintenance.Retainer), satisfied by the in-process client via type +// assertion at the composition root — the same shape as maintenance.Cleaner. +// +// The remote transport does not carry them and never will. Every retention +// batch is a cross-tenant DELETE by age; a route that answered one would let a +// fleet host delete rows from every workspace, which is exactly the capability +// docs/security.md invariant 73 says this seam may never express. Not +// implementing the interface is how registerScheduled is told so (see +// internal/coreapi/remote/controlplane.go). + +// RetentionRequest is one bounded batch of a retention sweep. +type RetentionRequest struct { + // OlderThan is the window: rows whose age is at least this are eligible. + // The cutoff is computed from the DATABASE clock, so replicas with skewed + // clocks agree on it. + OlderThan time.Duration + // Limit bounds the rows the batch SCANS past the cursor — and so the rows it + // can delete, and the length of its one transaction. Scanning, not deleting, + // is what is bounded: a batch whose rows are all kept by a guard costs the + // same as one that deletes them all. + Limit int32 + // After resumes strictly after the last row a previous batch scanned. The + // zero value starts from the oldest row. + After RetentionCursor +} + +// RetentionCursor is a position in a table's (age, id) order. The zero value is +// before every row. +type RetentionCursor struct { + At time.Time + ID uuid.UUID +} + +// RetentionBatch is what one batch did. +type RetentionBatch struct { + // Scanned is how many rows past the cursor the batch examined, kept or + // deleted. Scanned < RetentionRequest.Limit means nothing is left past the + // cursor: the table is drained. + Scanned int64 + // Deleted is how many of those rows of the table itself were removed. + Deleted int64 + // Dependents counts rows removed from OTHER tables along with them, so the + // sweep's metrics are honest about what a delete cost: the messages of a + // deleted inbox thread, the tracking events and rollups of a deleted send. + // Zero where a table has no dependents. + Dependents int64 + // RolledUp is how many tracking_event_rollups rows the batch inserted or + // incremented. Tracking events only; zero everywhere else. + RolledUp int64 + // Next is the cursor to resume from: the last row SCANNED. Meaningful only + // when Scanned > 0. + Next RetentionCursor +} + +// RetentionProgress is where a table's sweep got to in a previous run. +type RetentionProgress struct { + // Cursor is the position to resume from; the zero value means "from the + // oldest row". + Cursor RetentionCursor + // CycleExpired reports that the table has been walked without starting over + // for more than a day. The sweep then starts over from the oldest row, so a + // row a guard stopped protecting behind the cursor is revisited. + CycleExpired bool +} diff --git a/internal/platform/config/config.go b/internal/platform/config/config.go index aa5b9d82..76cde5e1 100644 --- a/internal/platform/config/config.go +++ b/internal/platform/config/config.go @@ -25,6 +25,11 @@ const ( DefaultDBMinConns = db.DefaultPoolMinConns ) +// DefaultRetentionDeadLettersDays is INROAD_RETENTION_DEAD_LETTERS_DAYS' default: +// the 90 days task_dead_letters was purged at, hard-coded, before the window +// became configurable — so an upgrade changes no deployment's behaviour. +const DefaultRetentionDeadLettersDays = 90 + type Config struct { Env string HTTPAddr string @@ -213,6 +218,27 @@ type Config struct { // means the default single-process topology. WorkerRole string + // Retention windows for the maintenance:retention sweep, in whole DAYS. Zero + // disables that table's sweep; it never means "keep nothing". + // + // The four recipient-data windows (sends; inbox threads with their + // messages; tracking events; deliverability events) default to 0 — + // DISABLED — and that default + // is a decision, not an omission: how long to keep data about the people a + // workspace emails is a Privacy/Legal call for each deployment, and a code + // default would be this project making it for them. Dead letters default to + // the 90 days they were purged at before the window was configurable. + // + // Only the shape is checked here (a whole number, not negative). Each table's + // MINIMUM window is domain knowledge — which rate reads it, over how long — so + // it lives with the sweep (internal/worker/maintenance.RetentionPolicy.Validate) + // and cmd/worker refuses to start below it. + RetentionSendsDays int + RetentionInboxDays int + RetentionTrackingEventsDays int + RetentionDeliverabilityEventsDays int + RetentionDeadLettersDays int + // --- Fleet credential brokering (internal/platform/credbroker) --- // // These three exist so a fleet worker can obtain decrypted mailbox @@ -560,6 +586,12 @@ func Load() (*Config, error) { cfg.RunScheduler = env.boolVal("INROAD_RUN_SCHEDULER", true) cfg.WorkerRole = os.Getenv("INROAD_WORKER_ROLE") + cfg.RetentionSendsDays = env.daysVal("INROAD_RETENTION_SENDS_DAYS", 0) + cfg.RetentionInboxDays = env.daysVal("INROAD_RETENTION_INBOX_DAYS", 0) + cfg.RetentionTrackingEventsDays = env.daysVal("INROAD_RETENTION_TRACKING_EVENTS_DAYS", 0) + cfg.RetentionDeliverabilityEventsDays = env.daysVal("INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS", 0) + cfg.RetentionDeadLettersDays = env.daysVal("INROAD_RETENTION_DEAD_LETTERS_DAYS", DefaultRetentionDeadLettersDays) + hostname, _ := os.Hostname() // "" on the rare lookup failure; handled below switch { case os.Getenv("INROAD_WORKER_ID") != "": @@ -777,6 +809,34 @@ func (r *envReader) intVal(key string, fallback int) int { return n } +// daysVal returns the whole number of days in key, or fallback when it is unset +// or blank. Zero is a real value (it disables a retention window), so it is +// accepted; a negative number is refused rather than clamped, because every +// variable this reads controls a DELETE and a sign typo must not be quietly +// reinterpreted as something the operator did not write. +// +// Days rather than a Go duration because a retention window is a policy stated +// in days ("keep sends for 400 days"), and time.ParseDuration has no day unit — +// an operator would otherwise have to write 9600h and hope they multiplied right. +func (r *envReader) daysVal(key string, fallback int) int { + v, ok := trimmedEnv(key) + if !ok { + return fallback + } + n, err := strconv.Atoi(v) + if err != nil || n < 0 || n > maxRetentionDays { + r.errs = append(r.errs, fmt.Errorf("%s must be a whole number of days from 0 (disabled) to %d, got %q", key, maxRetentionDays, v)) + return fallback + } + return n +} + +// maxRetentionDays caps a retention window at a century. Not a policy opinion: +// the window becomes a time.Duration downstream, which overflows past ~292 years +// and would wrap to a NEGATIVE window, and a window longer than a century is a +// disabled one that says so less clearly than 0 does. +const maxRetentionDays = 36500 + // durationVal returns the Go duration in key, or fallback when it is unset or // blank. The unit is not optional: INROAD_ACCESS_TOKEN_TTL=5 is not five of // anything, and used to be discarded in favour of a 5-minute default that made diff --git a/internal/platform/config/retention_test.go b/internal/platform/config/retention_test.go new file mode 100644 index 00000000..7eabdd24 --- /dev/null +++ b/internal/platform/config/retention_test.go @@ -0,0 +1,95 @@ +package config + +import ( + "strings" + "testing" +) + +// The defaults are the policy decision this feature is built around: every +// recipient-identifying table is DISABLED unless an operator turns it on, and +// dead letters keep the 90 days they were purged at before the window became +// configurable. A default that drifted to a number would start deleting data +// about people on the next deploy of every installation that never asked for it. +func TestRetentionDefaults(t *testing.T) { + setRequiredSecrets(t) + cfg, err := Load() + if err != nil { + t.Fatalf("Load(): %v", err) + } + for name, got := range map[string]int{ + "INROAD_RETENTION_SENDS_DAYS": cfg.RetentionSendsDays, + "INROAD_RETENTION_INBOX_DAYS": cfg.RetentionInboxDays, + "INROAD_RETENTION_TRACKING_EVENTS_DAYS": cfg.RetentionTrackingEventsDays, + "INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS": cfg.RetentionDeliverabilityEventsDays, + } { + if got != 0 { + t.Errorf("%s defaults to %d, want 0 (disabled — a Privacy/Legal decision, not a code default)", name, got) + } + } + if cfg.RetentionDeadLettersDays != 90 { + t.Errorf("INROAD_RETENTION_DEAD_LETTERS_DAYS defaults to %d, want 90 (the pre-existing hard-coded window)", cfg.RetentionDeadLettersDays) + } +} + +// Each variable reaches its own field — a transposed pair would apply one +// table's window to another and pass every "is it parsed" test. +func TestRetentionOverridesReachTheirOwnField(t *testing.T) { + setRequiredSecrets(t) + t.Setenv("INROAD_RETENTION_SENDS_DAYS", "401") + t.Setenv("INROAD_RETENTION_INBOX_DAYS", "402") + t.Setenv("INROAD_RETENTION_TRACKING_EVENTS_DAYS", "403") + t.Setenv("INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS", "404") + t.Setenv("INROAD_RETENTION_DEAD_LETTERS_DAYS", "0") + cfg, err := Load() + if err != nil { + t.Fatalf("Load(): %v", err) + } + got := [5]int{cfg.RetentionSendsDays, cfg.RetentionInboxDays, cfg.RetentionTrackingEventsDays, + cfg.RetentionDeliverabilityEventsDays, cfg.RetentionDeadLettersDays} + if want := [5]int{401, 402, 403, 404, 0}; got != want { + t.Fatalf("retention days = %v, want %v (0 must be honoured as 'disabled', not replaced by the default)", got, want) + } +} + +// Every variable here controls a DELETE, so a malformed value must stop the +// process rather than be read as something the operator did not write (#224). +// Negative is refused, not clamped to "disabled"; a unit is refused because the +// unit is fixed; an absurd window is refused because it would overflow a +// time.Duration downstream and wrap negative. +func TestRetentionRejectsMalformedDays(t *testing.T) { + for _, tc := range []struct { + name, key, value string + }{ + {"negative", "INROAD_RETENTION_SENDS_DAYS", "-30"}, + {"duration syntax", "INROAD_RETENTION_INBOX_DAYS", "720h"}, + {"unit suffix", "INROAD_RETENTION_TRACKING_EVENTS_DAYS", "90d"}, + {"word", "INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS", "forever"}, + {"fraction", "INROAD_RETENTION_DEAD_LETTERS_DAYS", "7.5"}, + {"beyond a century", "INROAD_RETENTION_SENDS_DAYS", "36501"}, + } { + t.Run(tc.name, func(t *testing.T) { + setRequiredSecrets(t) + t.Setenv(tc.key, tc.value) + _, err := Load() + if err == nil { + t.Fatalf("Load() with %s=%q returned nil error", tc.key, tc.value) + } + if !strings.Contains(err.Error(), tc.key) || !strings.Contains(err.Error(), tc.value) { + t.Fatalf("Load() error = %q, want it to name %s and quote %q", err, tc.key, tc.value) + } + }) + } +} + +// The ceiling itself is accepted — the boundary is inclusive. +func TestRetentionAcceptsTheCeiling(t *testing.T) { + setRequiredSecrets(t) + t.Setenv("INROAD_RETENTION_SENDS_DAYS", "36500") + cfg, err := Load() + if err != nil { + t.Fatalf("Load(): %v", err) + } + if cfg.RetentionSendsDays != 36500 { + t.Fatalf("RetentionSendsDays = %d, want 36500", cfg.RetentionSendsDays) + } +} diff --git a/internal/platform/db/concurrentindex_test.go b/internal/platform/db/concurrentindex_test.go new file mode 100644 index 00000000..61825b8c --- /dev/null +++ b/internal/platform/db/concurrentindex_test.go @@ -0,0 +1,45 @@ +package db + +import ( + "io/fs" + "regexp" + "strings" + "testing" +) + +var concurrently = regexp.MustCompile(`(?i)\bCONCURRENTLY\b`) + +// A migration that builds or drops an index CONCURRENTLY must be exactly ONE +// statement. golang-migrate's pgx/v5 driver sends a file as a single Exec; with +// one statement that is not a transaction block, and the concurrent build is +// allowed. With two, Postgres runs the file as an implicit transaction and +// refuses the CONCURRENTLY — at deploy time, on the production database, with +// the schema left dirty. This fails the build instead. +// +// TestRetentionMigrationsRollBackAndForwardAgain (integration) proves the other +// half: that such a file really does build, and leaves a VALID index. +func TestConcurrentIndexMigrationsAreSingleStatements(t *testing.T) { + names, err := fs.Glob(migrationsFS, "migrations/*.sql") + if err != nil { + t.Fatalf("glob migrations: %v", err) + } + var checked int + for _, name := range names { + raw, err := fs.ReadFile(migrationsFS, name) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + sql := strings.TrimSpace(stripComments(string(raw))) + if !concurrently.MatchString(sql) { + continue + } + checked++ + if n := strings.Count(sql, ";"); n != 1 || !strings.HasSuffix(sql, ";") { + t.Errorf("%s uses CONCURRENTLY but holds %d statements; it must be exactly one, or golang-migrate runs it "+ + "in an implicit transaction and Postgres refuses the concurrent build", name, n) + } + } + if checked == 0 { + t.Fatal("no CONCURRENTLY migration found — the scan is not reading the migrations it thinks it is") + } +} diff --git a/internal/platform/db/gen/campaign.sql.go b/internal/platform/db/gen/campaign.sql.go index 2af8d4e4..28a25131 100644 --- a/internal/platform/db/gen/campaign.sql.go +++ b/internal/platform/db/gen/campaign.sql.go @@ -326,13 +326,13 @@ WITH sent AS ( ), opened AS ( SELECT campaign_id, COUNT(DISTINCT send_id)::bigint AS n - FROM tracking_events + FROM tracking_engagement WHERE workspace_id = $1 AND kind = 'open' AND NOT is_machine GROUP BY 1 ), clicked AS ( SELECT campaign_id, COUNT(DISTINCT send_id)::bigint AS n - FROM tracking_events + FROM tracking_engagement WHERE workspace_id = $1 AND kind = 'click' AND NOT is_machine GROUP BY 1 ), @@ -407,6 +407,8 @@ type ListCampaignPerformanceRow struct { // click side gained a bot filter it never had: before this, a link scanner's // prefetch counted as a click here even while the open side filtered proxies, // so a scanned campaign could report more clicks than opens. +// tracking_engagement includes events retention has rolled up (migration +// 20260923110315), so this ranking agrees with CountEngagedSendsByKind. // One pass over the enrollments gives both the denominator (every enrollment // row is one contact, for the campaign's lifetime) and the stop-reason counts. func (q *Queries) ListCampaignPerformance(ctx context.Context, workspaceID uuid.UUID) ([]ListCampaignPerformanceRow, error) { diff --git a/internal/platform/db/gen/contact.sql.go b/internal/platform/db/gen/contact.sql.go index fb9bf96a..d1cf2d3d 100644 --- a/internal/platform/db/gen/contact.sql.go +++ b/internal/platform/db/gen/contact.sql.go @@ -138,8 +138,8 @@ SELECT count(DISTINCT te.send_id) FILTER ( count(DISTINCT te.send_id) FILTER ( WHERE te.kind = 'click' AND NOT te.is_machine )::bigint AS clicks, - max(te.created_at)::timestamptz AS last_event_at -FROM tracking_events te + max(te.last_at)::timestamptz AS last_event_at +FROM tracking_engagement te JOIN sends s ON s.id = te.send_id AND s.workspace_id = te.workspace_id WHERE te.workspace_id = $1 AND s.contact_id = $2 ` diff --git a/internal/platform/db/gen/deadletter.sql.go b/internal/platform/db/gen/deadletter.sql.go index 80eb50fe..c2c4dcff 100644 --- a/internal/platform/db/gen/deadletter.sql.go +++ b/internal/platform/db/gen/deadletter.sql.go @@ -267,18 +267,42 @@ func (q *Queries) ListTaskDeadLetters(ctx context.Context, arg ListTaskDeadLette } const purgeTaskDeadLetters = `-- name: PurgeTaskDeadLetters :one -WITH deleted AS ( - DELETE FROM task_dead_letters - WHERE id IN ( - SELECT id FROM task_dead_letters - WHERE created_at < now() - interval '90 days' - ORDER BY created_at LIMIT 5000 - ) - RETURNING 1 +WITH doomed AS ( + SELECT id, created_at + FROM task_dead_letters + WHERE created_at < now() - make_interval(secs => $1::bigint) + AND (created_at, id) > ($2::timestamptz, $3::uuid) + ORDER BY created_at, id + LIMIT $4::int + FOR UPDATE SKIP LOCKED +), +deleted AS ( + DELETE FROM task_dead_letters t + USING doomed d + WHERE t.id = d.id + RETURNING t.id, t.created_at +), +last_row AS ( + SELECT created_at, id FROM deleted ORDER BY created_at DESC, id DESC LIMIT 1 ) -SELECT count(*)::bigint AS deleted_rows FROM deleted +SELECT (SELECT count(*) FROM deleted)::bigint AS deleted_rows, + COALESCE((SELECT created_at FROM last_row), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_row), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id ` +type PurgeTaskDeadLettersParams struct { + OlderThanSeconds int64 `json:"older_than_seconds"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + BatchLimit int32 `json:"batch_limit"` +} + +type PurgeTaskDeadLettersRow struct { + DeletedRows int64 `json:"deleted_rows"` + LastAt pgtype.Timestamptz `json:"last_at"` + LastID uuid.UUID `json:"last_id"` +} + // Bound the table. task_dead_letters is append-only from the application's point // of view (triage flips a status, it never deletes) and it was in NO maintenance // sweep at all, so it grew forever — one row per permanently failed background @@ -287,25 +311,35 @@ SELECT count(*)::bigint AS deleted_rows FROM deleted // // Invariant 55's reasoning applies unchanged: warmup_observations was bounded at // 90 days because it is append-only and written by events nobody schedules, and -// this is the same shape. 90 days is comfortably beyond any triage window — a -// dropped send nobody has looked at in three months is not going to be replayed -// — and it keeps the audit answer migration 000069 exists for ("what did we -// re-run last week") intact by a wide margin. +// this is the same shape. 90 days — INROAD_RETENTION_DEAD_LETTERS_DAYS' default — +// is comfortably beyond any triage window: a dropped send nobody has looked at in +// three months is not going to be replayed, and it keeps the audit answer +// migration 000069 exists for ("what did we re-run last week") intact by a wide +// margin. // -// Batched at 5000 rows like every other purge in queries/maintenance.sql, to cap -// one sweep's lock/IO footprint. Deleting oldest-first (ORDER BY created_at) -// means repeated sweeps make monotonic progress rather than re-reading the same -// head of the table. +// Every status is eligible, 'pending' included, and that is deliberate: the +// table's indexes are not partial on 'pending' because terminal rows ARE the +// audit trail, and the window is what bounds how long that trail is kept. A +// 'pending' row past the window is work nobody triaged in the whole window. +// +// One bounded batch of the retention sweep (queries/retention.sql has the shared +// shape: database clock, SKIP LOCKED for overlapping replicas, a keyset cursor). +// Oldest-first on idx_task_dead_letters_created. // // Global (no workspace pin), for the same reason PurgeExpiredSecurityArtifacts // is: retention is deployment maintenance, not a tenant read. It removes rows by // age alone and returns only a count, so it can neither surface nor cross tenant // data. -func (q *Queries) PurgeTaskDeadLetters(ctx context.Context) (int64, error) { - row := q.db.QueryRow(ctx, purgeTaskDeadLetters) - var deleted_rows int64 - err := row.Scan(&deleted_rows) - return deleted_rows, err +func (q *Queries) PurgeTaskDeadLetters(ctx context.Context, arg PurgeTaskDeadLettersParams) (PurgeTaskDeadLettersRow, error) { + row := q.db.QueryRow(ctx, purgeTaskDeadLetters, + arg.OlderThanSeconds, + arg.AfterAt, + arg.AfterID, + arg.BatchLimit, + ) + var i PurgeTaskDeadLettersRow + err := row.Scan(&i.DeletedRows, &i.LastAt, &i.LastID) + return i, err } const releaseTaskDeadLetterReplay = `-- name: ReleaseTaskDeadLetterReplay :execrows diff --git a/internal/platform/db/gen/models.go b/internal/platform/db/gen/models.go index 3e644554..63fe2231 100644 --- a/internal/platform/db/gen/models.go +++ b/internal/platform/db/gen/models.go @@ -898,6 +898,14 @@ type ReplyLabel struct { UpdatedAt pgtype.Timestamptz `json:"updated_at"` } +type RetentionCursor struct { + TableName string `json:"table_name"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + CycleStartedAt pgtype.Timestamptz `json:"cycle_started_at"` + UpdatedAt pgtype.Timestamptz `json:"updated_at"` +} + type ScheduledJobRun struct { ID uuid.UUID `json:"id"` JobName string `json:"job_name"` @@ -1049,6 +1057,17 @@ type TaskTarget struct { CreatedAt pgtype.Timestamptz `json:"created_at"` } +type TrackingEngagement struct { + WorkspaceID uuid.UUID `json:"workspace_id"` + CampaignID uuid.UUID `json:"campaign_id"` + SendID uuid.UUID `json:"send_id"` + Kind TrackingEventKind `json:"kind"` + IsMachine bool `json:"is_machine"` + Events int64 `json:"events"` + FirstAt pgtype.Timestamptz `json:"first_at"` + LastAt pgtype.Timestamptz `json:"last_at"` +} + type TrackingEvent struct { ID uuid.UUID `json:"id"` WorkspaceID uuid.UUID `json:"workspace_id"` @@ -1063,6 +1082,17 @@ type TrackingEvent struct { ClientIp *netip.Addr `json:"client_ip"` } +type TrackingEventRollup struct { + WorkspaceID uuid.UUID `json:"workspace_id"` + CampaignID uuid.UUID `json:"campaign_id"` + SendID uuid.UUID `json:"send_id"` + Kind TrackingEventKind `json:"kind"` + IsMachine bool `json:"is_machine"` + Events int64 `json:"events"` + FirstAt pgtype.Timestamptz `json:"first_at"` + LastAt pgtype.Timestamptz `json:"last_at"` +} + type TwoFactorChallenge struct { ID uuid.UUID `json:"id"` UserID uuid.UUID `json:"user_id"` diff --git a/internal/platform/db/gen/results.sql.go b/internal/platform/db/gen/results.sql.go index 415091b7..3fc70b53 100644 --- a/internal/platform/db/gen/results.sql.go +++ b/internal/platform/db/gen/results.sql.go @@ -92,12 +92,12 @@ SELECT s.step_order, FROM sends s LEFT JOIN ( SELECT DISTINCT te.send_id - FROM tracking_events te + FROM tracking_engagement te WHERE te.workspace_id = $2 AND te.campaign_id = $1 AND te.kind = 'open' AND NOT te.is_machine ) o ON o.send_id = s.id LEFT JOIN ( SELECT DISTINCT te.send_id - FROM tracking_events te + FROM tracking_engagement te WHERE te.workspace_id = $2 AND te.campaign_id = $1 AND te.kind = 'click' AND NOT te.is_machine ) c ON c.send_id = s.id WHERE s.workspace_id = $2 AND s.campaign_id = $1 AND s.status = 'sent' @@ -147,6 +147,9 @@ type CampaignSendResultsRow struct { // Columns stay te-qualified inside each subquery: without the join to sends // these are self-contained, but an unqualified campaign_id then resolves against // the OUTER sends and the reference is ambiguous. +// +// tracking_engagement includes events retention has rolled up (migration +// 20260923110315), so a variant breakdown still sums to the campaign total. func (q *Queries) CampaignSendResults(ctx context.Context, arg CampaignSendResultsParams) ([]CampaignSendResultsRow, error) { rows, err := q.db.Query(ctx, campaignSendResults, arg.CampaignID, arg.WorkspaceID) if err != nil { diff --git a/internal/platform/db/gen/retention.sql.go b/internal/platform/db/gen/retention.sql.go new file mode 100644 index 00000000..8566404e --- /dev/null +++ b/internal/platform/db/gen/retention.sql.go @@ -0,0 +1,599 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: retention.sql + +package gen + +import ( + "context" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" +) + +const getRetentionCursor = `-- name: GetRetentionCursor :one +SELECT after_at, after_id, (cycle_started_at < now() - interval '24 hours')::boolean AS cycle_expired +FROM retention_cursors +WHERE table_name = $1 +` + +type GetRetentionCursorRow struct { + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + CycleExpired bool `json:"cycle_expired"` +} + +// Where a table's sweep got to. cycle_expired is computed on the database clock +// so the "once a day, start over" rule does not depend on which worker asks. +func (q *Queries) GetRetentionCursor(ctx context.Context, tableName string) (GetRetentionCursorRow, error) { + row := q.db.QueryRow(ctx, getRetentionCursor, tableName) + var i GetRetentionCursorRow + err := row.Scan(&i.AfterAt, &i.AfterID, &i.CycleExpired) + return i, err +} + +const purgeDeliverabilityEvents = `-- name: PurgeDeliverabilityEvents :one +WITH scanned AS MATERIALIZED ( + SELECT d.id, d.received_at + FROM deliverability_events d + WHERE d.received_at < now() - make_interval(secs => $1::bigint) + AND (d.received_at, d.id) > ($2::timestamptz, $3::uuid) + ORDER BY d.received_at, d.id + LIMIT $4::int +), +doomed AS ( + SELECT d.id + FROM deliverability_events d + JOIN scanned c ON c.id = d.id + WHERE NOT EXISTS ( + SELECT 1 + FROM sends s + JOIN campaigns cam ON cam.id = s.campaign_id AND cam.workspace_id = s.workspace_id + WHERE s.id = d.send_id AND s.workspace_id = d.workspace_id + AND cam.status IN ('running', 'paused') + AND d.received_at >= cam.guardrails_enabled_at + OFFSET 0 + ) + AND NOT ( + d.kind = 'complaint' + AND NOT EXISTS ( + SELECT 1 FROM deliverability_events n + WHERE n.workspace_id = d.workspace_id AND n.kind = 'complaint' + AND (n.received_at, n.id) > (d.received_at, d.id) + OFFSET 0 + ) + ) + FOR UPDATE OF d SKIP LOCKED +), +deleted AS ( + DELETE FROM deliverability_events d + USING doomed x + WHERE d.id = x.id + RETURNING 1 +), +last_scanned AS ( + SELECT received_at, id FROM scanned ORDER BY received_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM scanned)::bigint AS scanned_rows, + (SELECT count(*) FROM deleted)::bigint AS deleted_rows, + COALESCE((SELECT received_at FROM last_scanned), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_scanned), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id +` + +type PurgeDeliverabilityEventsParams struct { + OlderThanSeconds int64 `json:"older_than_seconds"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + BatchLimit int32 `json:"batch_limit"` +} + +type PurgeDeliverabilityEventsRow struct { + ScannedRows int64 `json:"scanned_rows"` + DeletedRows int64 `json:"deleted_rows"` + LastAt pgtype.Timestamptz `json:"last_at"` + LastID uuid.UUID `json:"last_id"` +} + +// Delete provider bounce/complaint events past the window, EXCEPT the two kinds +// of row a rate still reads. +// +// (1) An event inside a running OR PAUSED campaign's breaker window. The +// +// breaker's rolling window is 7 days, but when a campaign has too few recent +// sends it falls back to "everything since supervision began" — +// campaigns.guardrails_enabled_at (internal/app/deliverability/service.go +// assessCampaign). That fallback is unbounded in age, so no fixed floor can +// protect it; this guard does. Paused counts as running because a paused +// campaign is one resume away from being assessed on exactly this history: +// its stopped-as-bounced enrollments survive (they are not in any sweep), so +// deleting the events and sends beside them while it was paused would +// leave a numerator with a shrunken denominator and trip the breaker the +// moment it resumed. The guard keeps a superset of what the breaker reads — +// its floor is the LATER of enabled_at and the last operator override. +// Warmup health (30 days) and the workspace rollup (7 days) are fixed +// windows, protected by the 90-day floor RetentionPolicy enforces. +// +// (2) The workspace's newest complaint. GetCampaignDeliverabilityCounts and +// +// GetWorkspaceDeliverabilityCounts report complaint_feed = "has a complaint +// feed EVER reported here", which separates measured-and-clean from NOT +// MEASURED. Deleting a workspace's last complaint would flip a live feed to +// "not measured". Keeping one row preserves that answer; (received_at, id) +// makes "newest" total. +// +// The dedup key (workspace_id, provider_event_id) goes with the row: a provider +// replaying an event after it was deleted would be ingested again, at its NEW +// received_at. Provider retry windows are hours to days; the 90-day floor makes +// that unreachable in practice. +func (q *Queries) PurgeDeliverabilityEvents(ctx context.Context, arg PurgeDeliverabilityEventsParams) (PurgeDeliverabilityEventsRow, error) { + row := q.db.QueryRow(ctx, purgeDeliverabilityEvents, + arg.OlderThanSeconds, + arg.AfterAt, + arg.AfterID, + arg.BatchLimit, + ) + var i PurgeDeliverabilityEventsRow + err := row.Scan( + &i.ScannedRows, + &i.DeletedRows, + &i.LastAt, + &i.LastID, + ) + return i, err +} + +const purgeInboxThreads = `-- name: PurgeInboxThreads :one +WITH scanned AS MATERIALIZED ( + SELECT t.id, t.last_message_at + FROM inbox_threads t + WHERE t.last_message_at < now() - make_interval(secs => $1::bigint) + AND (t.last_message_at, t.id) > ($2::timestamptz, $3::uuid) + ORDER BY t.last_message_at, t.id + LIMIT $4::int +), +doomed AS ( + SELECT t.id + FROM inbox_threads t + JOIN scanned c ON c.id = t.id + WHERE NOT EXISTS ( + SELECT 1 FROM inbox_pending_replies p + WHERE p.thread_id = t.id AND p.workspace_id = t.workspace_id + AND p.status IN ('scheduled', 'sending') + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM inbox_thread_snoozes z + WHERE z.thread_id = t.id AND z.workspace_id = t.workspace_id + AND z.snooze_until > now() + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM sequence_enrollments e + WHERE e.campaign_id = t.campaign_id AND e.contact_id = t.contact_id + AND e.workspace_id = t.workspace_id AND e.status = 'active' + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM inbox_messages m + WHERE m.thread_id = t.id AND m.workspace_id = t.workspace_id + AND m.occurred_at >= now() - make_interval(secs => $1::bigint) + OFFSET 0 + ) + FOR UPDATE OF t SKIP LOCKED +), +deleted_messages AS ( + DELETE FROM inbox_messages m + USING doomed d + WHERE m.thread_id = d.id + RETURNING 1 +), +deleted_threads AS ( + DELETE FROM inbox_threads t + USING doomed d + WHERE t.id = d.id + RETURNING 1 +), +last_scanned AS ( + SELECT last_message_at, id FROM scanned ORDER BY last_message_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM scanned)::bigint AS scanned_rows, + (SELECT count(*) FROM deleted_threads)::bigint AS deleted_rows, + (SELECT count(*) FROM deleted_messages)::bigint AS deleted_messages, + COALESCE((SELECT last_message_at FROM last_scanned), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_scanned), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id +` + +type PurgeInboxThreadsParams struct { + OlderThanSeconds int64 `json:"older_than_seconds"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + BatchLimit int32 `json:"batch_limit"` +} + +type PurgeInboxThreadsRow struct { + ScannedRows int64 `json:"scanned_rows"` + DeletedRows int64 `json:"deleted_rows"` + DeletedMessages int64 `json:"deleted_messages"` + LastAt pgtype.Timestamptz `json:"last_at"` + LastID uuid.UUID `json:"last_id"` +} + +// Delete whole inbox conversations with no activity inside the window, messages +// and all. +// +// The unit is the THREAD, not the message. Deleting old messages one by one +// would leave a conversation with its opening missing and its later replies +// intact, which an operator would read as corruption, and the thread's +// synthesized outbound leg (from sends) would no longer line up with the +// replies to it. A thread is eligible when its last_message_at — bumped by every +// inbound reply and manual send — is past the window, i.e. the whole +// conversation is. +// +// Kept regardless of age: +// - a thread with a manual reply still scheduled or sending — the operator's +// own queued words; deleting the thread would cascade them away unsent; +// - a thread snoozed into the future — the operator asked to be shown it again; +// - a thread whose campaign enrollment is still active — the sequence is still +// writing to this conversation (its steps are its outbound leg); +// - a thread with any message inside the window, belt and braces for a writer +// that did not bump last_message_at. +// +// Labels, snoozes and finished pending replies cascade with the thread (the +// pending-reply cascade seeks idx_inbox_pending_replies_thread). +// +// The messages are deleted explicitly, in their own CTE, rather than left to the +// thread FK's ON DELETE CASCADE, so the count is real. The cascade then finds +// nothing left to do. A thread's messages come with it whatever their number, +// which is what keeps a conversation whole. +func (q *Queries) PurgeInboxThreads(ctx context.Context, arg PurgeInboxThreadsParams) (PurgeInboxThreadsRow, error) { + row := q.db.QueryRow(ctx, purgeInboxThreads, + arg.OlderThanSeconds, + arg.AfterAt, + arg.AfterID, + arg.BatchLimit, + ) + var i PurgeInboxThreadsRow + err := row.Scan( + &i.ScannedRows, + &i.DeletedRows, + &i.DeletedMessages, + &i.LastAt, + &i.LastID, + ) + return i, err +} + +const purgeSends = `-- name: PurgeSends :one +WITH scanned AS MATERIALIZED ( + SELECT s.id, s.created_at + FROM sends s + WHERE s.created_at < now() - make_interval(secs => $1::bigint) + AND (s.created_at, s.id) > ($2::timestamptz, $3::uuid) + ORDER BY s.created_at, s.id + LIMIT $4::int +), +doomed AS ( + SELECT s.id + FROM sends s + JOIN scanned c ON c.id = s.id + WHERE (s.sent_at IS NULL OR s.sent_at < now() - make_interval(secs => $1::bigint)) + AND s.status IN ('sent', 'failed', 'skipped') + AND NOT EXISTS ( + SELECT 1 FROM sequence_enrollments e + WHERE e.campaign_id = s.campaign_id AND e.contact_id = s.contact_id + AND e.workspace_id = s.workspace_id AND e.status = 'active' + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM campaigns cam + WHERE cam.id = s.campaign_id AND cam.workspace_id = s.workspace_id + AND cam.status IN ('running', 'paused') + AND s.sent_at >= cam.guardrails_enabled_at + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM inbox_threads t + WHERE t.workspace_id = s.workspace_id AND t.campaign_id = s.campaign_id + AND t.contact_id = s.contact_id + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM deliverability_events d + WHERE d.send_id = s.id AND d.workspace_id = s.workspace_id + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM deals dl + WHERE dl.workspace_id = s.workspace_id AND dl.primary_contact_id = s.contact_id + AND dl.source_campaign_id = s.campaign_id + OFFSET 0 + ) + FOR UPDATE OF s SKIP LOCKED +), +deleted_tracking AS ( + DELETE FROM tracking_events te + USING doomed d + WHERE te.send_id = d.id + RETURNING 1 +), +deleted_rollups AS ( + DELETE FROM tracking_event_rollups r + USING doomed d + WHERE r.send_id = d.id + RETURNING 1 +), +deleted_sends AS ( + DELETE FROM sends s + USING doomed d + WHERE s.id = d.id + RETURNING 1 +), +last_scanned AS ( + SELECT created_at, id FROM scanned ORDER BY created_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM scanned)::bigint AS scanned_rows, + (SELECT count(*) FROM deleted_sends)::bigint AS deleted_rows, + ((SELECT count(*) FROM deleted_tracking) + (SELECT count(*) FROM deleted_rollups))::bigint AS deleted_tracking, + COALESCE((SELECT created_at FROM last_scanned), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_scanned), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id +` + +type PurgeSendsParams struct { + OlderThanSeconds int64 `json:"older_than_seconds"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + BatchLimit int32 `json:"batch_limit"` +} + +type PurgeSendsRow struct { + ScannedRows int64 `json:"scanned_rows"` + DeletedRows int64 `json:"deleted_rows"` + DeletedTracking int64 `json:"deleted_tracking"` + LastAt pgtype.Timestamptz `json:"last_at"` + LastID uuid.UUID `json:"last_id"` +} + +// Delete campaign sends past the window that nothing live still depends on. +// +// Deleting a send removes it from EVERY report — sent counts, per-step and +// per-variant results, outcome attribution, the deliverability series — and +// with it its tracking events and rollups, so a rate's numerator and +// denominator shrink together rather than the rate drifting. That is what +// "retention" means for this table, and it is why it is off unless an operator +// turns it on. +// +// Kept regardless of age, each guard naming the reader it protects: +// - status queued/sending — work in flight; the row IS the send claim +// (stepsend.sql ClaimStepSend); +// - an ACTIVE enrollment for (campaign, contact) — the sequence engine threads +// its next step off earlier sends (LatestSentForContact: In-Reply-To and +// References), branches on their engagement, and stops on a reply matched +// through sends.message_id (GetSendByMessageID); +// - a RUNNING OR PAUSED campaign's send on or after guardrails_enabled_at — +// the breaker's fallback denominator (see PurgeDeliverabilityEvents for why +// paused counts): a campaign that can still run keeps its history; +// - an inbox thread for (campaign, contact) — the thread's outbound leg is +// synthesized from these rows (ListSentOutboundStepsForThread) and its +// "who spoke last" rule reads them (inbox_thread_awaiting_reply). The thread +// has its own retention; once it is gone the send is free; +// - a deliverability event naming it — the FK is ON DELETE SET NULL, which +// would silently strip the event of its campaign and mailbox attribution +// while the event itself is still retained; +// - a CRM deal sourced from (campaign, contact) — the deal's activity feed +// lists the campaign's sent messages (crm integration_store ListEvents). +// +// NOT a guard, deliberately: suppression. Nothing in suppression references a +// send, and an unsubscribe token carries (workspace, email), not a send id, so +// old unsubscribe links keep working. What does stop working is an old OPEN +// PIXEL or CLICK LINK — the tracking endpoint returns 404 for a send that is +// gone — and matching a late reply or DSN back to the campaign, which goes +// through sends.message_id. +// +// The tracking events and rollups are deleted explicitly, before the FK cascade +// would, so the count reported is real. Those deletes take no SKIP LOCKED: a +// concurrent RollupTrackingEvents holding those rows is excluded by the sweep's +// single-instance lock, not by this statement. +func (q *Queries) PurgeSends(ctx context.Context, arg PurgeSendsParams) (PurgeSendsRow, error) { + row := q.db.QueryRow(ctx, purgeSends, + arg.OlderThanSeconds, + arg.AfterAt, + arg.AfterID, + arg.BatchLimit, + ) + var i PurgeSendsRow + err := row.Scan( + &i.ScannedRows, + &i.DeletedRows, + &i.DeletedTracking, + &i.LastAt, + &i.LastID, + ) + return i, err +} + +const releaseRetentionSweepLock = `-- name: ReleaseRetentionSweepLock :one +SELECT pg_advisory_unlock(hashtext('inroad:maintenance:retention'))::boolean AS released +` + +func (q *Queries) ReleaseRetentionSweepLock(ctx context.Context) (bool, error) { + row := q.db.QueryRow(ctx, releaseRetentionSweepLock) + var released bool + err := row.Scan(&released) + return released, err +} + +const rollupTrackingEvents = `-- name: RollupTrackingEvents :one + +WITH doomed AS ( + SELECT te.id, te.created_at + FROM tracking_events te + WHERE te.created_at < now() - make_interval(secs => $1::bigint) + AND (te.created_at, te.id) > ($2::timestamptz, $3::uuid) + ORDER BY te.created_at, te.id + LIMIT $4::int + FOR UPDATE SKIP LOCKED +), +deleted AS ( + DELETE FROM tracking_events te + USING doomed d + WHERE te.id = d.id AND te.created_at = d.created_at + RETURNING te.id, te.workspace_id, te.campaign_id, te.send_id, te.kind, te.is_machine, te.created_at +), +rolled AS ( + INSERT INTO tracking_event_rollups AS r + (workspace_id, campaign_id, send_id, kind, is_machine, events, first_at, last_at) + SELECT (array_agg(workspace_id))[1], (array_agg(campaign_id))[1], send_id, kind, is_machine, + count(*), min(created_at), max(created_at) + FROM deleted + GROUP BY send_id, kind, is_machine + ON CONFLICT (send_id, kind, is_machine) DO UPDATE + SET events = r.events + EXCLUDED.events, + first_at = LEAST(r.first_at, EXCLUDED.first_at), + last_at = GREATEST(r.last_at, EXCLUDED.last_at) + RETURNING 1 +), +last_row AS ( + SELECT created_at, id FROM deleted ORDER BY created_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM deleted)::bigint AS deleted_rows, + (SELECT count(*) FROM rolled)::bigint AS rollup_rows, + COALESCE((SELECT created_at FROM last_row), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_row), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id +` + +type RollupTrackingEventsParams struct { + OlderThanSeconds int64 `json:"older_than_seconds"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + BatchLimit int32 `json:"batch_limit"` +} + +type RollupTrackingEventsRow struct { + DeletedRows int64 `json:"deleted_rows"` + RollupRows int64 `json:"rollup_rows"` + LastAt pgtype.Timestamptz `json:"last_at"` + LastID uuid.UUID `json:"last_id"` +} + +// Configurable retention for the recipient-identifying tables (migration +// 20260923110315). Each query is ONE bounded batch; the loop, the per-run budget, +// the persisted cursor and the enable/disable decision live in +// internal/worker/maintenance/retention.go. +// +// Shared shape, and why: +// +// - The window arrives as seconds and the cutoff is computed from the +// DATABASE's now(), never from a worker clock — replicas with skewed clocks +// must agree on what "older than 400 days" means. +// - A batch is bounded by the rows it SCANS, not the rows it deletes. The +// guarded tables first take the next `batch_limit` rows past the cursor in +// (age, id) order — an index range scan of fixed length — and only then apply +// the guards to those candidates. The cursor returned is the last row +// SCANNED, kept or not, so a run never re-reads a guard-kept row, and a batch +// whose candidates are all kept still costs one bounded scan instead of +// walking every kept row in the table looking for a deletable one. Bounding +// deletes alone was the earlier shape, and on a table whose oldest million +// rows are all kept it made a single statement read all of them. +// - Each guard is a NOT EXISTS ... OFFSET 0. The OFFSET 0 is load-bearing: it +// stops the planner pulling the sublink up into an anti-join, which it would +// otherwise happily do as a HASH anti-join over the entire guard table (every +// inbox thread, every enrollment) once per batch. As a SubPlan each guard is +// one index probe per candidate, which is bounded by the batch size. +// - Candidates are locked FOR UPDATE SKIP LOCKED, so a row the live path holds +// (a send mid-claim) is skipped rather than waited on. The sweep itself is +// single-instance across replicas (TryRetentionSweepLock). +// - The zero cursor (year 1, the nil UUID) is before every row. +// - Global (no workspace pin), for the same reason as every purge in +// maintenance.sql: retention is deployment maintenance, not a tenant read. +// Each query deletes by age and guard alone and returns only counts and a +// cursor, so it can neither surface nor cross tenant data. Every join a +// guard makes is pinned on workspace_id on both sides. +// +// The caller runs each of these inside a transaction with SET LOCAL +// statement_timeout (inprocess/retention.go), so no batch can outlive its +// budget however the plan turns out. +// Fold raw tracking events past the window into tracking_event_rollups, then +// they are gone. See the migration for what the rollup keeps and what it drops. +// +// The rollup is fed from the DELETE's own RETURNING, in the same statement, +// which is what makes it exactly-once: a row is counted if and only if THIS +// statement deleted it. A failed statement rolls back its delete and its +// increment together. Reading the rows first and deleting them second, in two +// statements, would double-count on any retry between the two. +// +// Scanned and deleted are the same rows here — there is no guard — so the +// LIMIT bounds both. +// +// Grouped by (send_id, kind, is_machine) — the conflict key — rather than by +// every carried column, because an INSERT whose SELECT produces two rows with +// one conflict key fails outright ("ON CONFLICT DO UPDATE command cannot affect +// row a second time") and would wedge the sweep on that batch forever. +// workspace_id is determined by send_id (the tenant FK to sends); campaign_id +// should be too, and taking one arbitrary value costs nothing if it ever is not. +func (q *Queries) RollupTrackingEvents(ctx context.Context, arg RollupTrackingEventsParams) (RollupTrackingEventsRow, error) { + row := q.db.QueryRow(ctx, rollupTrackingEvents, + arg.OlderThanSeconds, + arg.AfterAt, + arg.AfterID, + arg.BatchLimit, + ) + var i RollupTrackingEventsRow + err := row.Scan( + &i.DeletedRows, + &i.RollupRows, + &i.LastAt, + &i.LastID, + ) + return i, err +} + +const saveRetentionCursor = `-- name: SaveRetentionCursor :exec +INSERT INTO retention_cursors (table_name, after_at, after_id) +VALUES ($1, $2, $3) +ON CONFLICT (table_name) DO UPDATE +SET after_at = EXCLUDED.after_at, + after_id = EXCLUDED.after_id, + cycle_started_at = CASE WHEN $4::boolean THEN now() ELSE retention_cursors.cycle_started_at END, + updated_at = now() +` + +type SaveRetentionCursorParams struct { + TableName string `json:"table_name"` + AfterAt pgtype.Timestamptz `json:"after_at"` + AfterID uuid.UUID `json:"after_id"` + NewCycle bool `json:"new_cycle"` +} + +// Record a table's position. new_cycle restarts the day's clock: the sweep sets +// it when it starts over from the oldest row (the table drained, or the cycle +// expired), so "a row a guard stopped protecting is revisited within a day" +// holds however large the backlog. +func (q *Queries) SaveRetentionCursor(ctx context.Context, arg SaveRetentionCursorParams) error { + _, err := q.db.Exec(ctx, saveRetentionCursor, + arg.TableName, + arg.AfterAt, + arg.AfterID, + arg.NewCycle, + ) + return err +} + +const tryRetentionSweepLock = `-- name: TryRetentionSweepLock :one +SELECT pg_try_advisory_lock(hashtext('inroad:maintenance:retention'))::boolean AS acquired +` + +// The single-sweeper lock. Session-level, taken on a connection the sweep holds +// for its whole run, so exactly one retention run proceeds deployment-wide +// however many replicas run the scheduler. Two concurrent runs were safe row by +// row (SKIP LOCKED) but not table by table: PurgeSends deletes tracking rows +// that a concurrent RollupTrackingEvents holds, while the rollup's insert needs +// a KEY SHARE on the very send PurgeSends has locked — a deadlock Postgres would +// resolve by aborting one of them every time they met. hashtext() turns the +// name into the lock key so it is greppable rather than a magic number. +func (q *Queries) TryRetentionSweepLock(ctx context.Context) (bool, error) { + row := q.db.QueryRow(ctx, tryRetentionSweepLock) + var acquired bool + err := row.Scan(&acquired) + return acquired, err +} diff --git a/internal/platform/db/gen/tracking.sql.go b/internal/platform/db/gen/tracking.sql.go index 49ac158d..ec42de71 100644 --- a/internal/platform/db/gen/tracking.sql.go +++ b/internal/platform/db/gen/tracking.sql.go @@ -15,7 +15,7 @@ import ( const countEngagedSendsByKind = `-- name: CountEngagedSendsByKind :many SELECT kind, count(DISTINCT send_id)::bigint AS n -FROM tracking_events +FROM tracking_engagement WHERE campaign_id = $1 AND workspace_id = $2 AND NOT is_machine GROUP BY kind ` @@ -37,6 +37,10 @@ type CountEngagedSendsByKindRow struct { // CountTrackingEventsByKindAndVerdict reports them, but a prefetch must never // reach the headline rate -- nor, once conditional branching ships, the signal a // branch reads. +// +// tracking_engagement, not tracking_events, in this and every aggregate below: +// the view includes the events retention has rolled up, so a campaign's rate does +// not fall the day the sweep first runs (migration 20260923110315). func (q *Queries) CountEngagedSendsByKind(ctx context.Context, arg CountEngagedSendsByKindParams) ([]CountEngagedSendsByKindRow, error) { rows, err := q.db.Query(ctx, countEngagedSendsByKind, arg.CampaignID, arg.WorkspaceID) if err != nil { @@ -59,7 +63,7 @@ func (q *Queries) CountEngagedSendsByKind(ctx context.Context, arg CountEngagedS const countHumanOpens = `-- name: CountHumanOpens :one SELECT count(DISTINCT send_id)::bigint -FROM tracking_events +FROM tracking_engagement WHERE campaign_id = $1 AND workspace_id = $2 AND kind = 'open' AND NOT is_machine ` @@ -113,6 +117,11 @@ type CountRecentSendOpensFromSubnetParams struct { // would silently narrow "203.0.113.0/24" to a single host and the rule would // never fire. netip.Prefix.String() produces exactly the CIDR literal cidr() // parses. +// +// The one reader that stays on the RAW table, and correctly: it needs client_ip, +// which the retention rollup deliberately does not keep, and it only ever looks +// back botfilter.BurstWindow (10 minutes) — far inside the shortest window the +// retention sweep accepts, so no row it could read has been rolled up. func (q *Queries) CountRecentSendOpensFromSubnet(ctx context.Context, arg CountRecentSendOpensFromSubnetParams) (int64, error) { row := q.db.QueryRow(ctx, countRecentSendOpensFromSubnet, arg.SendID, arg.CreatedAt, arg.Subnet) var column_1 int64 @@ -122,7 +131,7 @@ func (q *Queries) CountRecentSendOpensFromSubnet(ctx context.Context, arg CountR const countTrackingEventsByKindAndVerdict = `-- name: CountTrackingEventsByKindAndVerdict :many SELECT kind, is_machine, count(DISTINCT send_id)::bigint AS n -FROM tracking_events +FROM tracking_engagement WHERE campaign_id = $1 AND workspace_id = $2 GROUP BY kind, is_machine ` @@ -162,9 +171,9 @@ func (q *Queries) CountTrackingEventsByKindAndVerdict(ctx context.Context, arg C } const getSendTrackingContext = `-- name: GetSendTrackingContext :one -SELECT COALESCE(count(*) FILTER (WHERE kind = 'open' AND NOT is_machine), 0)::bigint AS human_opens, - COALESCE(max(created_at) FILTER (WHERE kind = 'open' AND NOT is_machine), 'epoch'::timestamptz)::timestamptz AS last_human_open_at -FROM tracking_events +SELECT COALESCE(sum(events) FILTER (WHERE kind = 'open' AND NOT is_machine), 0)::bigint AS human_opens, + COALESCE(max(last_at) FILTER (WHERE kind = 'open' AND NOT is_machine), 'epoch'::timestamptz)::timestamptz AS last_human_open_at +FROM tracking_engagement WHERE send_id = $1 ` @@ -183,6 +192,14 @@ type GetSendTrackingContextRow struct { // about this same send, and no row content is returned. Scoping it would require // trusting a workspace id from an unauthenticated request, which is worse. // +// Read from tracking_engagement (migration 20260923110315), not tracking_events: +// once retention has rolled a send's old opens up, the raw table alone would tell +// the classifier "no human has ever opened this", and the ordering rule would +// judge the next open as if it were the first. Summing the rollup's `events` +// keeps the count the rule reads exactly what it was before the rollup; a raw +// row contributes 1. Both arms are index probes on send_id (idx_tracking_send_recent +// and the rollup's primary key). +// // Every aggregate is explicitly cast: FILTER/COALESCE aggregates otherwise // generate interface{} in the sqlc model and still compile. func (q *Queries) GetSendTrackingContext(ctx context.Context, sendID uuid.UUID) (GetSendTrackingContextRow, error) { diff --git a/internal/platform/db/migrations/20260923110315_recipient_data_retention.down.sql b/internal/platform/db/migrations/20260923110315_recipient_data_retention.down.sql new file mode 100644 index 00000000..b261b501 --- /dev/null +++ b/internal/platform/db/migrations/20260923110315_recipient_data_retention.down.sql @@ -0,0 +1,11 @@ +-- Reverse of the up file. +-- +-- Dropping tracking_event_rollups DESTROYS every rolled-up engagement count: a +-- campaign's historical open/click numbers drop to whatever raw tracking_events +-- rows are still inside the window. Nothing can recompute them, because the raw +-- rows they came from were deleted when they were rolled up. Run this only on a +-- deployment whose retention sweep has never rolled anything up, or accept that +-- loss knowingly. +DROP TABLE IF EXISTS retention_cursors; +DROP VIEW IF EXISTS tracking_engagement; +DROP TABLE IF EXISTS tracking_event_rollups; diff --git a/internal/platform/db/migrations/20260923110315_recipient_data_retention.up.sql b/internal/platform/db/migrations/20260923110315_recipient_data_retention.up.sql new file mode 100644 index 00000000..f75f19f6 --- /dev/null +++ b/internal/platform/db/migrations/20260923110315_recipient_data_retention.up.sql @@ -0,0 +1,121 @@ +-- Configurable retention for the five recipient-identifying tables — sends, +-- tracking_events, deliverability_events, inbox_threads and inbox_messages — +-- under four windows (a thread and its messages share one). The sweep is +-- maintenance:retention +-- (internal/worker/maintenance/retention.go); its windows are +-- INROAD_RETENTION_*_DAYS, and the recipient-data ones default to DISABLED — +-- how long to keep data about the people a workspace emails is a Privacy/Legal +-- decision, not a code default (docs: deploy/environment-variables). +-- +-- This file holds ONLY what is instant: two new, empty tables and a view. The +-- age indexes the sweep seeks on are each in a file of their own +-- (20260923144743..20260923144749), built CONCURRENTLY, so no write to sends or +-- tracking_events waits on an index build; 20260923144750 tunes autovacuum. +-- +-- LOCKING, stated exactly because it was stated wrongly before. golang-migrate +-- sends this whole file as one simple-protocol Exec, which Postgres runs as ONE +-- implicit transaction. Creating tracking_event_rollups' foreign keys takes SHARE +-- ROW EXCLUSIVE on sends, campaigns and workspaces, and that lock is held until +-- this file's transaction commits. Nothing else in the file scans or builds +-- anything — the new table is empty, the view is catalog-only — so the window is +-- milliseconds. That is also why no index on a populated table may ever be added +-- to this file: it would build while those three locks were held, blocking every +-- send claim and every workspace write for the length of the build. + +-- --------------------------------------------------------------------------- +-- tracking_event_rollups: the per-send summary a raw tracking event folds into +-- when it ages out. +-- +-- The aggregate readers ask one of three questions, and all three are +-- answerable from (send, kind, verdict) plus a count and a time range: +-- * "how many distinct sends had >= 1 human open/click" — every campaign, +-- contact and per-variant rate (COUNT(DISTINCT send_id)); +-- * "how many human opens has THIS send had, and when was the last one" — the +-- bot classifier's ordering rule (GetSendTrackingContext); +-- * "when did anything last happen to this contact's mail" (ContactTrackingStats). +-- So the grain is exactly one row per (send_id, kind, is_machine): nothing those +-- reports read is lost, and what none of them reads — the user agent, the client +-- IP (personal data about the recipient, see 20260827190215) and the click URL — +-- is not carried over. Historical numbers survive; per-hit detail about a person +-- does not. +-- +-- ANY query that reads tracking events must read tracking_engagement (below), +-- not tracking_events, or it silently loses every event older than the tracking +-- window. The exceptions are pinned by an allowlist test +-- (internal/platform/db/trackingreaders_test.go): the classifier's 10-minute +-- burst rule (needs client_ip, far inside any window), the CRM deal activity feed +-- (an event log, which a rollup cannot preserve — rolled-up events leave it), and +-- the sandbox seeder (writes). +-- +-- is_machine is frozen at rollup time. A future backfill that RECLASSIFIES raw +-- events (a new botfilter rule applied to history) cannot reach a rolled-up row: +-- the evidence it would re-judge — the user agent and IP — is exactly what the +-- rollup dropped. Such a backfill covers raw rows only, and must say so. +-- +-- fillfactor 90: the ON CONFLICT DO UPDATE in RollupTrackingEvents rewrites a +-- send's row each time more of its events age out; free space on the page lets +-- that be a HOT update rather than a new tuple plus index entries. +CREATE TABLE IF NOT EXISTS tracking_event_rollups ( + workspace_id UUID NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, + campaign_id UUID NOT NULL, + send_id UUID NOT NULL, + kind tracking_event_kind NOT NULL, + is_machine BOOLEAN NOT NULL, + events BIGINT NOT NULL CHECK (events > 0), + first_at TIMESTAMPTZ NOT NULL, + last_at TIMESTAMPTZ NOT NULL, + CHECK (first_at <= last_at), + PRIMARY KEY (send_id, kind, is_machine), + -- The same two tenant FKs tracking_events carries (000028), with the same + -- cascade: a rollup is the send's engagement, so when the send goes — a + -- sends retention delete, a contact or campaign deletion — its engagement + -- goes with it and a rate's numerator and denominator shrink together. + FOREIGN KEY (send_id, workspace_id) REFERENCES sends(id, workspace_id) ON DELETE CASCADE, + FOREIGN KEY (campaign_id, workspace_id) REFERENCES campaigns(id, workspace_id) ON DELETE CASCADE +) WITH (fillfactor = 90); +-- The campaign aggregations' access path, mirroring idx_tracking_campaign_kind on +-- the raw table so the rollup arm of every campaign rate is index-only too. +-- Plain CREATE INDEX is right here and only here: the table was created empty a +-- statement ago. +CREATE INDEX IF NOT EXISTS idx_tracking_rollups_campaign_kind + ON tracking_event_rollups (campaign_id, workspace_id, kind, is_machine, send_id); +-- ListCampaignPerformance aggregates a whole workspace by (kind, verdict) and +-- GROUPs BY campaign_id; carrying campaign_id ahead of send_id lets that be +-- index-only. The PK leads with send_id, which that query does not have. +CREATE INDEX IF NOT EXISTS idx_tracking_rollups_workspace_kind + ON tracking_event_rollups (workspace_id, kind, is_machine, campaign_id, send_id); + +-- tracking_engagement is the ONE definition of "every tracking event, rolled up +-- or not". A plain UNION ALL view: Postgres pushes each reader's WHERE into both +-- arms, so each keeps its own index path. It holds no data and needs no refresh. +-- A raw row is one event whose first and last sighting are the same instant. +CREATE OR REPLACE VIEW tracking_engagement AS +SELECT workspace_id, campaign_id, send_id, kind, is_machine, + 1::bigint AS events, + created_at AS first_at, + created_at AS last_at +FROM tracking_events +UNION ALL +SELECT workspace_id, campaign_id, send_id, kind, is_machine, + events, first_at, last_at +FROM tracking_event_rollups; + +-- --------------------------------------------------------------------------- +-- retention_cursors: where each table's sweep got to, across runs. +-- +-- Without it every run restarts from the oldest row, and on a table whose oldest +-- rows are KEPT by a guard (sends of long-lived active enrollments, a running +-- campaign's history) a run can spend its whole batch budget re-reading kept rows +-- and never reach the deletable ones behind them. The sweep resumes from here and +-- starts over from the oldest row when a table drains, or once a day +-- (cycle_started_at), so a row a guard stopped protecting is reached again. +-- +-- Deployment infrastructure, not tenant data: one row per swept table, holding a +-- position in (age, id) order and nothing about any row's content. +CREATE TABLE IF NOT EXISTS retention_cursors ( + table_name TEXT PRIMARY KEY CHECK (table_name <> ''), + after_at TIMESTAMPTZ NOT NULL, + after_id UUID NOT NULL, + cycle_started_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); diff --git a/internal/platform/db/migrations/20260923144743_tracking_events_created_at_index.down.sql b/internal/platform/db/migrations/20260923144743_tracking_events_created_at_index.down.sql new file mode 100644 index 00000000..4f421213 --- /dev/null +++ b/internal/platform/db/migrations/20260923144743_tracking_events_created_at_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +DROP INDEX CONCURRENTLY IF EXISTS idx_tracking_events_created_at; diff --git a/internal/platform/db/migrations/20260923144743_tracking_events_created_at_index.up.sql b/internal/platform/db/migrations/20260923144743_tracking_events_created_at_index.up.sql new file mode 100644 index 00000000..4dcc087a --- /dev/null +++ b/internal/platform/db/migrations/20260923144743_tracking_events_created_at_index.up.sql @@ -0,0 +1,22 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: RollupTrackingEvents walks tracking_events oldest-first by +-- (created_at, id) with no workspace predicate; every existing index leads with +-- send_id, campaign_id or workspace_id, so without this each batch was a Seq +-- Scan plus a Sort of the largest table in the schema. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_tracking_events_created_at + ON tracking_events (created_at, id); diff --git a/internal/platform/db/migrations/20260923144744_deliverability_events_received_at_index.down.sql b/internal/platform/db/migrations/20260923144744_deliverability_events_received_at_index.down.sql new file mode 100644 index 00000000..ac0e3946 --- /dev/null +++ b/internal/platform/db/migrations/20260923144744_deliverability_events_received_at_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +DROP INDEX CONCURRENTLY IF EXISTS idx_deliverability_events_received_at; diff --git a/internal/platform/db/migrations/20260923144744_deliverability_events_received_at_index.up.sql b/internal/platform/db/migrations/20260923144744_deliverability_events_received_at_index.up.sql new file mode 100644 index 00000000..0e9a7486 --- /dev/null +++ b/internal/platform/db/migrations/20260923144744_deliverability_events_received_at_index.up.sql @@ -0,0 +1,21 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: PurgeDeliverabilityEvents walks deliverability_events oldest-first by +-- (received_at, id) with no workspace predicate; the existing indexes lead with +-- workspace_id or send_id. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_deliverability_events_received_at + ON deliverability_events (received_at, id); diff --git a/internal/platform/db/migrations/20260923144745_inbox_threads_last_message_at_index.down.sql b/internal/platform/db/migrations/20260923144745_inbox_threads_last_message_at_index.down.sql new file mode 100644 index 00000000..0cb068ad --- /dev/null +++ b/internal/platform/db/migrations/20260923144745_inbox_threads_last_message_at_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +DROP INDEX CONCURRENTLY IF EXISTS idx_inbox_threads_last_message_at; diff --git a/internal/platform/db/migrations/20260923144745_inbox_threads_last_message_at_index.up.sql b/internal/platform/db/migrations/20260923144745_inbox_threads_last_message_at_index.up.sql new file mode 100644 index 00000000..e7d85589 --- /dev/null +++ b/internal/platform/db/migrations/20260923144745_inbox_threads_last_message_at_index.up.sql @@ -0,0 +1,21 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: PurgeInboxThreads walks inbox_threads oldest-activity-first by +-- (last_message_at, id) with no workspace predicate; the existing indexes lead +-- with workspace_id or mailbox_id. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_inbox_threads_last_message_at + ON inbox_threads (last_message_at, id); diff --git a/internal/platform/db/migrations/20260923144746_sends_created_at_index.down.sql b/internal/platform/db/migrations/20260923144746_sends_created_at_index.down.sql new file mode 100644 index 00000000..d014db80 --- /dev/null +++ b/internal/platform/db/migrations/20260923144746_sends_created_at_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +DROP INDEX CONCURRENTLY IF EXISTS idx_sends_created_at; diff --git a/internal/platform/db/migrations/20260923144746_sends_created_at_index.up.sql b/internal/platform/db/migrations/20260923144746_sends_created_at_index.up.sql new file mode 100644 index 00000000..fa58b4d2 --- /dev/null +++ b/internal/platform/db/migrations/20260923144746_sends_created_at_index.up.sql @@ -0,0 +1,20 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: PurgeSends walks sends oldest-first by (created_at, id) with no +-- workspace predicate; the only created_at index is partial on status='queued'. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_sends_created_at + ON sends (created_at, id); diff --git a/internal/platform/db/migrations/20260923144747_inbox_threads_campaign_contact_index.down.sql b/internal/platform/db/migrations/20260923144747_inbox_threads_campaign_contact_index.down.sql new file mode 100644 index 00000000..ba605d33 --- /dev/null +++ b/internal/platform/db/migrations/20260923144747_inbox_threads_campaign_contact_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +DROP INDEX CONCURRENTLY IF EXISTS idx_inbox_threads_campaign_contact; diff --git a/internal/platform/db/migrations/20260923144747_inbox_threads_campaign_contact_index.up.sql b/internal/platform/db/migrations/20260923144747_inbox_threads_campaign_contact_index.up.sql new file mode 100644 index 00000000..6c9e0797 --- /dev/null +++ b/internal/platform/db/migrations/20260923144747_inbox_threads_campaign_contact_index.up.sql @@ -0,0 +1,23 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: PurgeSends keeps any send an inbox thread still renders — the thread's +-- outbound leg is synthesized from sends by (workspace_id, campaign_id, +-- contact_id) — and probes inbox_threads by exactly that key once per candidate +-- send. Nothing on inbox_threads indexed campaign_id or contact_id. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_inbox_threads_campaign_contact + ON inbox_threads (workspace_id, campaign_id, contact_id) + WHERE campaign_id IS NOT NULL; diff --git a/internal/platform/db/migrations/20260923144748_inbox_pending_replies_thread_index.down.sql b/internal/platform/db/migrations/20260923144748_inbox_pending_replies_thread_index.down.sql new file mode 100644 index 00000000..51e954f0 --- /dev/null +++ b/internal/platform/db/migrations/20260923144748_inbox_pending_replies_thread_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +DROP INDEX CONCURRENTLY IF EXISTS idx_inbox_pending_replies_thread; diff --git a/internal/platform/db/migrations/20260923144748_inbox_pending_replies_thread_index.up.sql b/internal/platform/db/migrations/20260923144748_inbox_pending_replies_thread_index.up.sql new file mode 100644 index 00000000..34c094fc --- /dev/null +++ b/internal/platform/db/migrations/20260923144748_inbox_pending_replies_thread_index.up.sql @@ -0,0 +1,23 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: deleting an inbox thread cascades into inbox_pending_replies through the +-- (thread_id, workspace_id) FK. The only thread_id index there is PARTIAL on the +-- live statuses, which the cascade's `WHERE thread_id = $1` cannot use, so every +-- retention-deleted thread cost a Seq Scan of the whole table — thousands of +-- them per batch. The retention guard itself still uses the partial index. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_inbox_pending_replies_thread + ON inbox_pending_replies (thread_id); diff --git a/internal/platform/db/migrations/20260923144749_drop_tracking_events_send_index.down.sql b/internal/platform/db/migrations/20260923144749_drop_tracking_events_send_index.down.sql new file mode 100644 index 00000000..33a5dda9 --- /dev/null +++ b/internal/platform/db/migrations/20260923144749_drop_tracking_events_send_index.down.sql @@ -0,0 +1,2 @@ +-- ONE STATEMENT; see the up file. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_tracking_events_send ON tracking_events (send_id); diff --git a/internal/platform/db/migrations/20260923144749_drop_tracking_events_send_index.up.sql b/internal/platform/db/migrations/20260923144749_drop_tracking_events_send_index.up.sql new file mode 100644 index 00000000..a4c83bb0 --- /dev/null +++ b/internal/platform/db/migrations/20260923144749_drop_tracking_events_send_index.up.sql @@ -0,0 +1,26 @@ +-- ONE STATEMENT, deliberately, and it must stay that way. golang-migrate's +-- pgx/v5 driver sends a migration file as a single simple-protocol Exec; with +-- one statement that is not a transaction block, so CREATE/DROP INDEX +-- CONCURRENTLY is allowed and the table keeps taking writes while the index +-- builds. A second statement in this file would turn it into an implicit +-- transaction and the CONCURRENTLY would be refused +-- (internal/platform/db/concurrentindex_test.go pins the one statement; +-- retention_integration_test.go asserts the build and pg_index.indisvalid). +-- +-- IF A BUILD FAILS (a crash, a cancelled deploy, a deadlock) Postgres leaves the +-- index behind marked INVALID, golang-migrate marks the schema dirty, and IF NOT +-- EXISTS would then skip the broken index forever. Recover with: +-- DROP INDEX CONCURRENTLY IF EXISTS ; +-- UPDATE schema_migrations SET version = , dirty = false; +-- and re-run the migrations. The deploy docs carry the same procedure. +-- +-- Why: idx_tracking_events_send (send_id) is redundant with +-- idx_tracking_send_recent (send_id, kind, is_machine, created_at DESC), which +-- leads with the same column. Every reader that finds tracking rows by send — +-- GetSendTrackingContext, CountRecentSendOpensFromSubnet, ContactTrackingStats' +-- join, the CRM deal feed's join, the sandbox seeder's NOT EXISTS, PurgeSends' +-- dependent delete and the sends FK's ON DELETE CASCADE — has send_id as its +-- leading predicate, which the wider index serves. Dropping it pays for +-- idx_tracking_events_created_at: tracking_events stays at the same number of +-- indexes to maintain on every pixel hit. +DROP INDEX CONCURRENTLY IF EXISTS idx_tracking_events_send; diff --git a/internal/platform/db/migrations/20260923144750_retention_autovacuum.down.sql b/internal/platform/db/migrations/20260923144750_retention_autovacuum.down.sql new file mode 100644 index 00000000..7b11e3ad --- /dev/null +++ b/internal/platform/db/migrations/20260923144750_retention_autovacuum.down.sql @@ -0,0 +1,4 @@ +-- Back to the server defaults. +ALTER TABLE deliverability_events RESET (autovacuum_vacuum_scale_factor, autovacuum_vacuum_threshold); +ALTER TABLE tracking_events RESET (autovacuum_vacuum_scale_factor, autovacuum_vacuum_threshold); +ALTER TABLE sends RESET (autovacuum_vacuum_scale_factor, autovacuum_vacuum_threshold); diff --git a/internal/platform/db/migrations/20260923144750_retention_autovacuum.up.sql b/internal/platform/db/migrations/20260923144750_retention_autovacuum.up.sql new file mode 100644 index 00000000..144fe9fc --- /dev/null +++ b/internal/platform/db/migrations/20260923144750_retention_autovacuum.up.sql @@ -0,0 +1,19 @@ +-- Autovacuum tuned for the tables the retention sweep deletes from in bulk. +-- +-- The default autovacuum_vacuum_scale_factor is 0.2: a table is vacuumed once a +-- FIFTH of it is dead. On a 100M-row sends or tracking_events table that is 20M +-- dead tuples before anything is reclaimed, and the sweep's own age-index scans +-- walk those dead entries until it is. A flat threshold plus 1% keeps vacuum +-- following the sweep in steps rather than one enormous pass. +-- +-- inbox_threads/inbox_messages are left at the defaults: they are orders of +-- magnitude smaller, and their deletes are whole conversations. +-- +-- ALTER TABLE ... SET (storage parameters) takes SHARE UPDATE EXCLUSIVE, which +-- does not block reads or writes, and rewrites nothing. Several statements, so +-- this file runs as one implicit transaction — fine, since none of them waits +-- on anything but concurrent DDL. The settings are inert on a deployment that +-- never enables retention (few dead tuples, so nothing changes). +ALTER TABLE sends SET (autovacuum_vacuum_scale_factor = 0.01, autovacuum_vacuum_threshold = 50000); +ALTER TABLE tracking_events SET (autovacuum_vacuum_scale_factor = 0.01, autovacuum_vacuum_threshold = 50000); +ALTER TABLE deliverability_events SET (autovacuum_vacuum_scale_factor = 0.01, autovacuum_vacuum_threshold = 50000); diff --git a/internal/platform/db/queries/campaign.sql b/internal/platform/db/queries/campaign.sql index 4d9ed73f..c1270731 100644 --- a/internal/platform/db/queries/campaign.sql +++ b/internal/platform/db/queries/campaign.sql @@ -172,15 +172,17 @@ WITH sent AS ( -- click side gained a bot filter it never had: before this, a link scanner's -- prefetch counted as a click here even while the open side filtered proxies, -- so a scanned campaign could report more clicks than opens. +-- tracking_engagement includes events retention has rolled up (migration +-- 20260923110315), so this ranking agrees with CountEngagedSendsByKind. opened AS ( SELECT campaign_id, COUNT(DISTINCT send_id)::bigint AS n - FROM tracking_events + FROM tracking_engagement WHERE workspace_id = $1 AND kind = 'open' AND NOT is_machine GROUP BY 1 ), clicked AS ( SELECT campaign_id, COUNT(DISTINCT send_id)::bigint AS n - FROM tracking_events + FROM tracking_engagement WHERE workspace_id = $1 AND kind = 'click' AND NOT is_machine GROUP BY 1 ), diff --git a/internal/platform/db/queries/contact.sql b/internal/platform/db/queries/contact.sql index 9b29266b..d361bfb0 100644 --- a/internal/platform/db/queries/contact.sql +++ b/internal/platform/db/queries/contact.sql @@ -159,8 +159,8 @@ SELECT count(DISTINCT te.send_id) FILTER ( count(DISTINCT te.send_id) FILTER ( WHERE te.kind = 'click' AND NOT te.is_machine )::bigint AS clicks, - max(te.created_at)::timestamptz AS last_event_at -FROM tracking_events te + max(te.last_at)::timestamptz AS last_event_at +FROM tracking_engagement te JOIN sends s ON s.id = te.send_id AND s.workspace_id = te.workspace_id WHERE te.workspace_id = $1 AND s.contact_id = $2; diff --git a/internal/platform/db/queries/deadletter.sql b/internal/platform/db/queries/deadletter.sql index c9db10ac..b161eff4 100644 --- a/internal/platform/db/queries/deadletter.sql +++ b/internal/platform/db/queries/deadletter.sql @@ -128,27 +128,43 @@ WHERE workspace_id = @workspace_id AND id = @id AND status = 'pending'; -- -- Invariant 55's reasoning applies unchanged: warmup_observations was bounded at -- 90 days because it is append-only and written by events nobody schedules, and --- this is the same shape. 90 days is comfortably beyond any triage window — a --- dropped send nobody has looked at in three months is not going to be replayed --- — and it keeps the audit answer migration 000069 exists for ("what did we --- re-run last week") intact by a wide margin. +-- this is the same shape. 90 days — INROAD_RETENTION_DEAD_LETTERS_DAYS' default — +-- is comfortably beyond any triage window: a dropped send nobody has looked at in +-- three months is not going to be replayed, and it keeps the audit answer +-- migration 000069 exists for ("what did we re-run last week") intact by a wide +-- margin. -- --- Batched at 5000 rows like every other purge in queries/maintenance.sql, to cap --- one sweep's lock/IO footprint. Deleting oldest-first (ORDER BY created_at) --- means repeated sweeps make monotonic progress rather than re-reading the same --- head of the table. +-- Every status is eligible, 'pending' included, and that is deliberate: the +-- table's indexes are not partial on 'pending' because terminal rows ARE the +-- audit trail, and the window is what bounds how long that trail is kept. A +-- 'pending' row past the window is work nobody triaged in the whole window. +-- +-- One bounded batch of the retention sweep (queries/retention.sql has the shared +-- shape: database clock, SKIP LOCKED for overlapping replicas, a keyset cursor). +-- Oldest-first on idx_task_dead_letters_created. -- -- Global (no workspace pin), for the same reason PurgeExpiredSecurityArtifacts -- is: retention is deployment maintenance, not a tenant read. It removes rows by -- age alone and returns only a count, so it can neither surface nor cross tenant -- data. -WITH deleted AS ( - DELETE FROM task_dead_letters - WHERE id IN ( - SELECT id FROM task_dead_letters - WHERE created_at < now() - interval '90 days' - ORDER BY created_at LIMIT 5000 - ) - RETURNING 1 +WITH doomed AS ( + SELECT id, created_at + FROM task_dead_letters + WHERE created_at < now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint) + AND (created_at, id) > (sqlc.arg(after_at)::timestamptz, sqlc.arg(after_id)::uuid) + ORDER BY created_at, id + LIMIT sqlc.arg(batch_limit)::int + FOR UPDATE SKIP LOCKED +), +deleted AS ( + DELETE FROM task_dead_letters t + USING doomed d + WHERE t.id = d.id + RETURNING t.id, t.created_at +), +last_row AS ( + SELECT created_at, id FROM deleted ORDER BY created_at DESC, id DESC LIMIT 1 ) -SELECT count(*)::bigint AS deleted_rows FROM deleted; +SELECT (SELECT count(*) FROM deleted)::bigint AS deleted_rows, + COALESCE((SELECT created_at FROM last_row), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_row), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id; diff --git a/internal/platform/db/queries/results.sql b/internal/platform/db/queries/results.sql index 28aabc21..dc88ef4a 100644 --- a/internal/platform/db/queries/results.sql +++ b/internal/platform/db/queries/results.sql @@ -35,14 +35,17 @@ FROM sends s -- Columns stay te-qualified inside each subquery: without the join to sends -- these are self-contained, but an unqualified campaign_id then resolves against -- the OUTER sends and the reference is ambiguous. +-- +-- tracking_engagement includes events retention has rolled up (migration +-- 20260923110315), so a variant breakdown still sums to the campaign total. LEFT JOIN ( SELECT DISTINCT te.send_id - FROM tracking_events te + FROM tracking_engagement te WHERE te.workspace_id = $2 AND te.campaign_id = $1 AND te.kind = 'open' AND NOT te.is_machine ) o ON o.send_id = s.id LEFT JOIN ( SELECT DISTINCT te.send_id - FROM tracking_events te + FROM tracking_engagement te WHERE te.workspace_id = $2 AND te.campaign_id = $1 AND te.kind = 'click' AND NOT te.is_machine ) c ON c.send_id = s.id WHERE s.workspace_id = $2 AND s.campaign_id = $1 AND s.status = 'sent' diff --git a/internal/platform/db/queries/retention.sql b/internal/platform/db/queries/retention.sql new file mode 100644 index 00000000..321a6d28 --- /dev/null +++ b/internal/platform/db/queries/retention.sql @@ -0,0 +1,402 @@ +-- Configurable retention for the recipient-identifying tables (migration +-- 20260923110315). Each query is ONE bounded batch; the loop, the per-run budget, +-- the persisted cursor and the enable/disable decision live in +-- internal/worker/maintenance/retention.go. +-- +-- Shared shape, and why: +-- +-- * The window arrives as seconds and the cutoff is computed from the +-- DATABASE's now(), never from a worker clock — replicas with skewed clocks +-- must agree on what "older than 400 days" means. +-- * A batch is bounded by the rows it SCANS, not the rows it deletes. The +-- guarded tables first take the next `batch_limit` rows past the cursor in +-- (age, id) order — an index range scan of fixed length — and only then apply +-- the guards to those candidates. The cursor returned is the last row +-- SCANNED, kept or not, so a run never re-reads a guard-kept row, and a batch +-- whose candidates are all kept still costs one bounded scan instead of +-- walking every kept row in the table looking for a deletable one. Bounding +-- deletes alone was the earlier shape, and on a table whose oldest million +-- rows are all kept it made a single statement read all of them. +-- * Each guard is a NOT EXISTS ... OFFSET 0. The OFFSET 0 is load-bearing: it +-- stops the planner pulling the sublink up into an anti-join, which it would +-- otherwise happily do as a HASH anti-join over the entire guard table (every +-- inbox thread, every enrollment) once per batch. As a SubPlan each guard is +-- one index probe per candidate, which is bounded by the batch size. +-- * Candidates are locked FOR UPDATE SKIP LOCKED, so a row the live path holds +-- (a send mid-claim) is skipped rather than waited on. The sweep itself is +-- single-instance across replicas (TryRetentionSweepLock). +-- * The zero cursor (year 1, the nil UUID) is before every row. +-- * Global (no workspace pin), for the same reason as every purge in +-- maintenance.sql: retention is deployment maintenance, not a tenant read. +-- Each query deletes by age and guard alone and returns only counts and a +-- cursor, so it can neither surface nor cross tenant data. Every join a +-- guard makes is pinned on workspace_id on both sides. +-- +-- The caller runs each of these inside a transaction with SET LOCAL +-- statement_timeout (inprocess/retention.go), so no batch can outlive its +-- budget however the plan turns out. + +-- name: RollupTrackingEvents :one +-- Fold raw tracking events past the window into tracking_event_rollups, then +-- they are gone. See the migration for what the rollup keeps and what it drops. +-- +-- The rollup is fed from the DELETE's own RETURNING, in the same statement, +-- which is what makes it exactly-once: a row is counted if and only if THIS +-- statement deleted it. A failed statement rolls back its delete and its +-- increment together. Reading the rows first and deleting them second, in two +-- statements, would double-count on any retry between the two. +-- +-- Scanned and deleted are the same rows here — there is no guard — so the +-- LIMIT bounds both. +-- +-- Grouped by (send_id, kind, is_machine) — the conflict key — rather than by +-- every carried column, because an INSERT whose SELECT produces two rows with +-- one conflict key fails outright ("ON CONFLICT DO UPDATE command cannot affect +-- row a second time") and would wedge the sweep on that batch forever. +-- workspace_id is determined by send_id (the tenant FK to sends); campaign_id +-- should be too, and taking one arbitrary value costs nothing if it ever is not. +WITH doomed AS ( + SELECT te.id, te.created_at + FROM tracking_events te + WHERE te.created_at < now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint) + AND (te.created_at, te.id) > (sqlc.arg(after_at)::timestamptz, sqlc.arg(after_id)::uuid) + ORDER BY te.created_at, te.id + LIMIT sqlc.arg(batch_limit)::int + FOR UPDATE SKIP LOCKED +), +deleted AS ( + DELETE FROM tracking_events te + USING doomed d + WHERE te.id = d.id AND te.created_at = d.created_at + RETURNING te.id, te.workspace_id, te.campaign_id, te.send_id, te.kind, te.is_machine, te.created_at +), +rolled AS ( + INSERT INTO tracking_event_rollups AS r + (workspace_id, campaign_id, send_id, kind, is_machine, events, first_at, last_at) + SELECT (array_agg(workspace_id))[1], (array_agg(campaign_id))[1], send_id, kind, is_machine, + count(*), min(created_at), max(created_at) + FROM deleted + GROUP BY send_id, kind, is_machine + ON CONFLICT (send_id, kind, is_machine) DO UPDATE + SET events = r.events + EXCLUDED.events, + first_at = LEAST(r.first_at, EXCLUDED.first_at), + last_at = GREATEST(r.last_at, EXCLUDED.last_at) + RETURNING 1 +), +last_row AS ( + SELECT created_at, id FROM deleted ORDER BY created_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM deleted)::bigint AS deleted_rows, + (SELECT count(*) FROM rolled)::bigint AS rollup_rows, + COALESCE((SELECT created_at FROM last_row), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_row), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id; + +-- name: PurgeDeliverabilityEvents :one +-- Delete provider bounce/complaint events past the window, EXCEPT the two kinds +-- of row a rate still reads. +-- +-- (1) An event inside a running OR PAUSED campaign's breaker window. The +-- breaker's rolling window is 7 days, but when a campaign has too few recent +-- sends it falls back to "everything since supervision began" — +-- campaigns.guardrails_enabled_at (internal/app/deliverability/service.go +-- assessCampaign). That fallback is unbounded in age, so no fixed floor can +-- protect it; this guard does. Paused counts as running because a paused +-- campaign is one resume away from being assessed on exactly this history: +-- its stopped-as-bounced enrollments survive (they are not in any sweep), so +-- deleting the events and sends beside them while it was paused would +-- leave a numerator with a shrunken denominator and trip the breaker the +-- moment it resumed. The guard keeps a superset of what the breaker reads — +-- its floor is the LATER of enabled_at and the last operator override. +-- Warmup health (30 days) and the workspace rollup (7 days) are fixed +-- windows, protected by the 90-day floor RetentionPolicy enforces. +-- +-- (2) The workspace's newest complaint. GetCampaignDeliverabilityCounts and +-- GetWorkspaceDeliverabilityCounts report complaint_feed = "has a complaint +-- feed EVER reported here", which separates measured-and-clean from NOT +-- MEASURED. Deleting a workspace's last complaint would flip a live feed to +-- "not measured". Keeping one row preserves that answer; (received_at, id) +-- makes "newest" total. +-- +-- The dedup key (workspace_id, provider_event_id) goes with the row: a provider +-- replaying an event after it was deleted would be ingested again, at its NEW +-- received_at. Provider retry windows are hours to days; the 90-day floor makes +-- that unreachable in practice. +WITH scanned AS MATERIALIZED ( + SELECT d.id, d.received_at + FROM deliverability_events d + WHERE d.received_at < now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint) + AND (d.received_at, d.id) > (sqlc.arg(after_at)::timestamptz, sqlc.arg(after_id)::uuid) + ORDER BY d.received_at, d.id + LIMIT sqlc.arg(batch_limit)::int +), +doomed AS ( + SELECT d.id + FROM deliverability_events d + JOIN scanned c ON c.id = d.id + WHERE NOT EXISTS ( + SELECT 1 + FROM sends s + JOIN campaigns cam ON cam.id = s.campaign_id AND cam.workspace_id = s.workspace_id + WHERE s.id = d.send_id AND s.workspace_id = d.workspace_id + AND cam.status IN ('running', 'paused') + AND d.received_at >= cam.guardrails_enabled_at + OFFSET 0 + ) + AND NOT ( + d.kind = 'complaint' + AND NOT EXISTS ( + SELECT 1 FROM deliverability_events n + WHERE n.workspace_id = d.workspace_id AND n.kind = 'complaint' + AND (n.received_at, n.id) > (d.received_at, d.id) + OFFSET 0 + ) + ) + FOR UPDATE OF d SKIP LOCKED +), +deleted AS ( + DELETE FROM deliverability_events d + USING doomed x + WHERE d.id = x.id + RETURNING 1 +), +last_scanned AS ( + SELECT received_at, id FROM scanned ORDER BY received_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM scanned)::bigint AS scanned_rows, + (SELECT count(*) FROM deleted)::bigint AS deleted_rows, + COALESCE((SELECT received_at FROM last_scanned), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_scanned), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id; + +-- name: PurgeInboxThreads :one +-- Delete whole inbox conversations with no activity inside the window, messages +-- and all. +-- +-- The unit is the THREAD, not the message. Deleting old messages one by one +-- would leave a conversation with its opening missing and its later replies +-- intact, which an operator would read as corruption, and the thread's +-- synthesized outbound leg (from sends) would no longer line up with the +-- replies to it. A thread is eligible when its last_message_at — bumped by every +-- inbound reply and manual send — is past the window, i.e. the whole +-- conversation is. +-- +-- Kept regardless of age: +-- * a thread with a manual reply still scheduled or sending — the operator's +-- own queued words; deleting the thread would cascade them away unsent; +-- * a thread snoozed into the future — the operator asked to be shown it again; +-- * a thread whose campaign enrollment is still active — the sequence is still +-- writing to this conversation (its steps are its outbound leg); +-- * a thread with any message inside the window, belt and braces for a writer +-- that did not bump last_message_at. +-- Labels, snoozes and finished pending replies cascade with the thread (the +-- pending-reply cascade seeks idx_inbox_pending_replies_thread). +-- +-- The messages are deleted explicitly, in their own CTE, rather than left to the +-- thread FK's ON DELETE CASCADE, so the count is real. The cascade then finds +-- nothing left to do. A thread's messages come with it whatever their number, +-- which is what keeps a conversation whole. +WITH scanned AS MATERIALIZED ( + SELECT t.id, t.last_message_at + FROM inbox_threads t + WHERE t.last_message_at < now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint) + AND (t.last_message_at, t.id) > (sqlc.arg(after_at)::timestamptz, sqlc.arg(after_id)::uuid) + ORDER BY t.last_message_at, t.id + LIMIT sqlc.arg(batch_limit)::int +), +doomed AS ( + SELECT t.id + FROM inbox_threads t + JOIN scanned c ON c.id = t.id + WHERE NOT EXISTS ( + SELECT 1 FROM inbox_pending_replies p + WHERE p.thread_id = t.id AND p.workspace_id = t.workspace_id + AND p.status IN ('scheduled', 'sending') + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM inbox_thread_snoozes z + WHERE z.thread_id = t.id AND z.workspace_id = t.workspace_id + AND z.snooze_until > now() + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM sequence_enrollments e + WHERE e.campaign_id = t.campaign_id AND e.contact_id = t.contact_id + AND e.workspace_id = t.workspace_id AND e.status = 'active' + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM inbox_messages m + WHERE m.thread_id = t.id AND m.workspace_id = t.workspace_id + AND m.occurred_at >= now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint) + OFFSET 0 + ) + FOR UPDATE OF t SKIP LOCKED +), +deleted_messages AS ( + DELETE FROM inbox_messages m + USING doomed d + WHERE m.thread_id = d.id + RETURNING 1 +), +deleted_threads AS ( + DELETE FROM inbox_threads t + USING doomed d + WHERE t.id = d.id + RETURNING 1 +), +last_scanned AS ( + SELECT last_message_at, id FROM scanned ORDER BY last_message_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM scanned)::bigint AS scanned_rows, + (SELECT count(*) FROM deleted_threads)::bigint AS deleted_rows, + (SELECT count(*) FROM deleted_messages)::bigint AS deleted_messages, + COALESCE((SELECT last_message_at FROM last_scanned), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_scanned), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id; + +-- name: PurgeSends :one +-- Delete campaign sends past the window that nothing live still depends on. +-- +-- Deleting a send removes it from EVERY report — sent counts, per-step and +-- per-variant results, outcome attribution, the deliverability series — and +-- with it its tracking events and rollups, so a rate's numerator and +-- denominator shrink together rather than the rate drifting. That is what +-- "retention" means for this table, and it is why it is off unless an operator +-- turns it on. +-- +-- Kept regardless of age, each guard naming the reader it protects: +-- * status queued/sending — work in flight; the row IS the send claim +-- (stepsend.sql ClaimStepSend); +-- * an ACTIVE enrollment for (campaign, contact) — the sequence engine threads +-- its next step off earlier sends (LatestSentForContact: In-Reply-To and +-- References), branches on their engagement, and stops on a reply matched +-- through sends.message_id (GetSendByMessageID); +-- * a RUNNING OR PAUSED campaign's send on or after guardrails_enabled_at — +-- the breaker's fallback denominator (see PurgeDeliverabilityEvents for why +-- paused counts): a campaign that can still run keeps its history; +-- * an inbox thread for (campaign, contact) — the thread's outbound leg is +-- synthesized from these rows (ListSentOutboundStepsForThread) and its +-- "who spoke last" rule reads them (inbox_thread_awaiting_reply). The thread +-- has its own retention; once it is gone the send is free; +-- * a deliverability event naming it — the FK is ON DELETE SET NULL, which +-- would silently strip the event of its campaign and mailbox attribution +-- while the event itself is still retained; +-- * a CRM deal sourced from (campaign, contact) — the deal's activity feed +-- lists the campaign's sent messages (crm integration_store ListEvents). +-- +-- NOT a guard, deliberately: suppression. Nothing in suppression references a +-- send, and an unsubscribe token carries (workspace, email), not a send id, so +-- old unsubscribe links keep working. What does stop working is an old OPEN +-- PIXEL or CLICK LINK — the tracking endpoint returns 404 for a send that is +-- gone — and matching a late reply or DSN back to the campaign, which goes +-- through sends.message_id. +-- +-- The tracking events and rollups are deleted explicitly, before the FK cascade +-- would, so the count reported is real. Those deletes take no SKIP LOCKED: a +-- concurrent RollupTrackingEvents holding those rows is excluded by the sweep's +-- single-instance lock, not by this statement. +WITH scanned AS MATERIALIZED ( + SELECT s.id, s.created_at + FROM sends s + WHERE s.created_at < now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint) + AND (s.created_at, s.id) > (sqlc.arg(after_at)::timestamptz, sqlc.arg(after_id)::uuid) + ORDER BY s.created_at, s.id + LIMIT sqlc.arg(batch_limit)::int +), +doomed AS ( + SELECT s.id + FROM sends s + JOIN scanned c ON c.id = s.id + WHERE (s.sent_at IS NULL OR s.sent_at < now() - make_interval(secs => sqlc.arg(older_than_seconds)::bigint)) + AND s.status IN ('sent', 'failed', 'skipped') + AND NOT EXISTS ( + SELECT 1 FROM sequence_enrollments e + WHERE e.campaign_id = s.campaign_id AND e.contact_id = s.contact_id + AND e.workspace_id = s.workspace_id AND e.status = 'active' + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM campaigns cam + WHERE cam.id = s.campaign_id AND cam.workspace_id = s.workspace_id + AND cam.status IN ('running', 'paused') + AND s.sent_at >= cam.guardrails_enabled_at + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM inbox_threads t + WHERE t.workspace_id = s.workspace_id AND t.campaign_id = s.campaign_id + AND t.contact_id = s.contact_id + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM deliverability_events d + WHERE d.send_id = s.id AND d.workspace_id = s.workspace_id + OFFSET 0 + ) + AND NOT EXISTS ( + SELECT 1 FROM deals dl + WHERE dl.workspace_id = s.workspace_id AND dl.primary_contact_id = s.contact_id + AND dl.source_campaign_id = s.campaign_id + OFFSET 0 + ) + FOR UPDATE OF s SKIP LOCKED +), +deleted_tracking AS ( + DELETE FROM tracking_events te + USING doomed d + WHERE te.send_id = d.id + RETURNING 1 +), +deleted_rollups AS ( + DELETE FROM tracking_event_rollups r + USING doomed d + WHERE r.send_id = d.id + RETURNING 1 +), +deleted_sends AS ( + DELETE FROM sends s + USING doomed d + WHERE s.id = d.id + RETURNING 1 +), +last_scanned AS ( + SELECT created_at, id FROM scanned ORDER BY created_at DESC, id DESC LIMIT 1 +) +SELECT (SELECT count(*) FROM scanned)::bigint AS scanned_rows, + (SELECT count(*) FROM deleted_sends)::bigint AS deleted_rows, + ((SELECT count(*) FROM deleted_tracking) + (SELECT count(*) FROM deleted_rollups))::bigint AS deleted_tracking, + COALESCE((SELECT created_at FROM last_scanned), '0001-01-01T00:00:00Z'::timestamptz)::timestamptz AS last_at, + COALESCE((SELECT id FROM last_scanned), '00000000-0000-0000-0000-000000000000'::uuid)::uuid AS last_id; + +-- name: GetRetentionCursor :one +-- Where a table's sweep got to. cycle_expired is computed on the database clock +-- so the "once a day, start over" rule does not depend on which worker asks. +SELECT after_at, after_id, (cycle_started_at < now() - interval '24 hours')::boolean AS cycle_expired +FROM retention_cursors +WHERE table_name = @table_name; + +-- name: SaveRetentionCursor :exec +-- Record a table's position. new_cycle restarts the day's clock: the sweep sets +-- it when it starts over from the oldest row (the table drained, or the cycle +-- expired), so "a row a guard stopped protecting is revisited within a day" +-- holds however large the backlog. +INSERT INTO retention_cursors (table_name, after_at, after_id) +VALUES (@table_name, @after_at, @after_id) +ON CONFLICT (table_name) DO UPDATE +SET after_at = EXCLUDED.after_at, + after_id = EXCLUDED.after_id, + cycle_started_at = CASE WHEN sqlc.arg(new_cycle)::boolean THEN now() ELSE retention_cursors.cycle_started_at END, + updated_at = now(); + +-- name: TryRetentionSweepLock :one +-- The single-sweeper lock. Session-level, taken on a connection the sweep holds +-- for its whole run, so exactly one retention run proceeds deployment-wide +-- however many replicas run the scheduler. Two concurrent runs were safe row by +-- row (SKIP LOCKED) but not table by table: PurgeSends deletes tracking rows +-- that a concurrent RollupTrackingEvents holds, while the rollup's insert needs +-- a KEY SHARE on the very send PurgeSends has locked — a deadlock Postgres would +-- resolve by aborting one of them every time they met. hashtext() turns the +-- name into the lock key so it is greppable rather than a magic number. +SELECT pg_try_advisory_lock(hashtext('inroad:maintenance:retention'))::boolean AS acquired; + +-- name: ReleaseRetentionSweepLock :one +SELECT pg_advisory_unlock(hashtext('inroad:maintenance:retention'))::boolean AS released; diff --git a/internal/platform/db/queries/tracking.sql b/internal/platform/db/queries/tracking.sql index 916758f9..5e984c6a 100644 --- a/internal/platform/db/queries/tracking.sql +++ b/internal/platform/db/queries/tracking.sql @@ -16,11 +16,19 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,sqlc.narg(client_ip)::inet); -- about this same send, and no row content is returned. Scoping it would require -- trusting a workspace id from an unauthenticated request, which is worse. -- +-- Read from tracking_engagement (migration 20260923110315), not tracking_events: +-- once retention has rolled a send's old opens up, the raw table alone would tell +-- the classifier "no human has ever opened this", and the ordering rule would +-- judge the next open as if it were the first. Summing the rollup's `events` +-- keeps the count the rule reads exactly what it was before the rollup; a raw +-- row contributes 1. Both arms are index probes on send_id (idx_tracking_send_recent +-- and the rollup's primary key). +-- -- Every aggregate is explicitly cast: FILTER/COALESCE aggregates otherwise -- generate interface{} in the sqlc model and still compile. -SELECT COALESCE(count(*) FILTER (WHERE kind = 'open' AND NOT is_machine), 0)::bigint AS human_opens, - COALESCE(max(created_at) FILTER (WHERE kind = 'open' AND NOT is_machine), 'epoch'::timestamptz)::timestamptz AS last_human_open_at -FROM tracking_events +SELECT COALESCE(sum(events) FILTER (WHERE kind = 'open' AND NOT is_machine), 0)::bigint AS human_opens, + COALESCE(max(last_at) FILTER (WHERE kind = 'open' AND NOT is_machine), 'epoch'::timestamptz)::timestamptz AS last_human_open_at +FROM tracking_engagement WHERE send_id = $1; -- name: CountRecentSendOpensFromSubnet :one @@ -38,6 +46,11 @@ WHERE send_id = $1; -- would silently narrow "203.0.113.0/24" to a single host and the rule would -- never fire. netip.Prefix.String() produces exactly the CIDR literal cidr() -- parses. +-- +-- The one reader that stays on the RAW table, and correctly: it needs client_ip, +-- which the retention rollup deliberately does not keep, and it only ever looks +-- back botfilter.BurstWindow (10 minutes) — far inside the shortest window the +-- retention sweep accepts, so no row it could read has been rolled up. SELECT count(*)::bigint FROM tracking_events WHERE send_id = $1 @@ -54,8 +67,12 @@ WHERE send_id = $1 -- CountTrackingEventsByKindAndVerdict reports them, but a prefetch must never -- reach the headline rate -- nor, once conditional branching ships, the signal a -- branch reads. +-- +-- tracking_engagement, not tracking_events, in this and every aggregate below: +-- the view includes the events retention has rolled up, so a campaign's rate does +-- not fall the day the sweep first runs (migration 20260923110315). SELECT kind, count(DISTINCT send_id)::bigint AS n -FROM tracking_events +FROM tracking_engagement WHERE campaign_id = $1 AND workspace_id = $2 AND NOT is_machine GROUP BY kind; @@ -64,7 +81,7 @@ GROUP BY kind; -- can say "N opens, M of them machine" instead of quietly showing the filtered -- number as if nothing had been excluded. SELECT kind, is_machine, count(DISTINCT send_id)::bigint AS n -FROM tracking_events +FROM tracking_engagement WHERE campaign_id = $1 AND workspace_id = $2 GROUP BY kind, is_machine; @@ -78,5 +95,5 @@ GROUP BY kind, is_machine; -- The verdict is now computed once, at write time, by platform/botfilter, and -- every reader agrees by construction. The join to sends is gone with it. SELECT count(DISTINCT send_id)::bigint -FROM tracking_events +FROM tracking_engagement WHERE campaign_id = $1 AND workspace_id = $2 AND kind = 'open' AND NOT is_machine; diff --git a/internal/platform/db/retention_integration_test.go b/internal/platform/db/retention_integration_test.go new file mode 100644 index 00000000..de8a0576 --- /dev/null +++ b/internal/platform/db/retention_integration_test.go @@ -0,0 +1,87 @@ +//go:build integration + +package db_test + +import ( + "context" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/inroad/inroad/internal/platform/db" + "github.com/inroad/inroad/internal/platform/db/dbtest" +) + +// The retention age indexes, each built by its own one-statement CONCURRENTLY +// migration (20260923144743..48). +var retentionIndexes = []string{ + "idx_tracking_events_created_at", "idx_deliverability_events_received_at", + "idx_inbox_threads_last_message_at", "idx_sends_created_at", + "idx_inbox_threads_campaign_contact", "idx_inbox_pending_replies_thread", +} + +// The retention migrations go up, down and up again cleanly on a database of +// their own (see dbtest.ScratchDSN for why a rollback never runs on the shared +// one). This is also the proof the CONCURRENTLY files rely on: golang-migrate's +// pgx/v5 driver sends a file as one Exec, so a one-statement file is NOT in a +// transaction block — were it, Postgres would refuse the build outright +// ("CREATE INDEX CONCURRENTLY cannot run inside a transaction block"). And a +// concurrent build that "succeeded" but left an INVALID index would be the worst +// outcome, so every index is asserted valid, not merely present. +func TestRetentionMigrationsRollBackAndForwardAgain(t *testing.T) { + ctx := context.Background() + dsn := dbtest.ScratchDSN(t, "retention_migration") + + if err := db.Migrate(dsn); err != nil { + t.Fatalf("migrate up (a CONCURRENTLY file inside a transaction block would fail here): %v", err) + } + pool, err := db.Connect(ctx, dsn) + if err != nil { + t.Fatalf("connect: %v", err) + } + defer pool.Close() // before the scratch DROP, which the t.Cleanup above runs after + + assertRetentionSchema(t, ctx, pool, true) + + // The version before the first retention migration. + if err := db.MigrateTo(dsn, 20260921111415); err != nil { + t.Fatalf("migrate down to 20260921111415: %v", err) + } + assertRetentionSchema(t, ctx, pool, false) + + if err := db.Migrate(dsn); err != nil { + t.Fatalf("migrate up again: %v", err) + } + assertRetentionSchema(t, ctx, pool, true) +} + +func assertRetentionSchema(t *testing.T, ctx context.Context, pool *pgxpool.Pool, up bool) { + t.Helper() + var tables, views, validIndexes, invalidIndexes, oldSendIndex, tunedTables int + if err := pool.QueryRow(ctx, ` + SELECT + (SELECT count(*) FROM pg_tables WHERE schemaname = 'public' AND tablename IN ('tracking_event_rollups', 'retention_cursors')), + (SELECT count(*) FROM pg_views WHERE schemaname = 'public' AND viewname = 'tracking_engagement'), + (SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE c.relname = ANY($1) AND i.indisvalid), + (SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE c.relname = ANY($1) AND NOT i.indisvalid), + (SELECT count(*) FROM pg_class WHERE relname = 'idx_tracking_events_send' AND relkind = 'i'), + (SELECT count(*) FROM pg_class WHERE relname IN ('sends', 'tracking_events', 'deliverability_events') + AND reloptions @> ARRAY['autovacuum_vacuum_scale_factor=0.01', 'autovacuum_vacuum_threshold=50000'])`, + retentionIndexes).Scan(&tables, &views, &validIndexes, &invalidIndexes, &oldSendIndex, &tunedTables); err != nil { + t.Fatalf("read catalog: %v", err) + } + if invalidIndexes != 0 { + t.Fatalf("%d retention index(es) are INVALID — a concurrent build failed and left a broken index behind", invalidIndexes) + } + if up { + if tables != 2 || views != 1 || validIndexes != len(retentionIndexes) || oldSendIndex != 0 || tunedTables != 3 { + t.Fatalf("after up: tables=%d view=%d valid indexes=%d/%d old send index=%d tuned tables=%d; want 2/1/%d/0/3", + tables, views, validIndexes, len(retentionIndexes), oldSendIndex, tunedTables, len(retentionIndexes)) + } + return + } + if tables != 0 || views != 0 || validIndexes != 0 || oldSendIndex != 1 || tunedTables != 0 { + t.Fatalf("after down: tables=%d view=%d indexes=%d old send index=%d tuned tables=%d; want 0/0/0/1 (restored)/0", + tables, views, validIndexes, oldSendIndex, tunedTables) + } +} diff --git a/internal/platform/db/retentionplan_integration_test.go b/internal/platform/db/retentionplan_integration_test.go new file mode 100644 index 00000000..3e908166 --- /dev/null +++ b/internal/platform/db/retentionplan_integration_test.go @@ -0,0 +1,190 @@ +//go:build integration + +package db_test + +import ( + "context" + _ "embed" + "fmt" + "regexp" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/inroad/inroad/internal/platform/db" + "github.com/inroad/inroad/internal/platform/db/dbtest" +) + +//go:embed queries/retention.sql +var retentionQueries string + +// sqlcArg matches sqlc's named-parameter syntax so the query text can be +// PREPAREd as Postgres sees it: each distinct name becomes $1, $2, … in order of +// first appearance, which is exactly what sqlc generates. +var sqlcArg = regexp.MustCompile(`sqlc\.arg\((\w+)\)`) + +// retentionQuery returns one named query from queries/retention.sql with its +// sqlc parameters rewritten to positional ones, and the parameter names in order. +func retentionQuery(t *testing.T, name string) (string, []string) { + t.Helper() + start := strings.Index(retentionQueries, "-- name: "+name+" ") + if start < 0 { + t.Fatalf("query %s not found in retention.sql", name) + } + body := retentionQueries[start:] + if next := strings.Index(body[1:], "-- name: "); next >= 0 { + body = body[:next+1] + } + var params []string + index := map[string]int{} + body = sqlcArg.ReplaceAllStringFunc(body, func(m string) string { + p := sqlcArg.FindStringSubmatch(m)[1] + if _, ok := index[p]; !ok { + params = append(params, p) + index[p] = len(params) + } + return fmt.Sprintf("$%d", index[p]) + }) + return strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(body), ";")), params +} + +// The sweep's guarded batches must stay bounded however the planner feels, and +// the way they could stop being bounded is a HASH anti-join: pull a guard's +// NOT EXISTS up into a join and the plan reads the whole guard table (every +// inbox thread, every enrollment, every deal) once per batch. The queries write +// each guard as NOT EXISTS ... OFFSET 0 to forbid that, and this pins it — on a +// GENERIC plan, the one a long-lived prepared statement ends up on, over enough +// data that the planner would otherwise choose the hash. +func TestRetentionBatchPlansProbeGuardsPerCandidate(t *testing.T) { + ctx := context.Background() + dsn := dbtest.DSN(t) + if err := db.Migrate(dsn); err != nil { + t.Fatalf("migrate: %v", err) + } + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(pool.Close) // registered first so it runs LAST, after the workspace delete + ws := seedRetentionVolume(t, ctx, pool) + t.Cleanup(func() { + if _, err := pool.Exec(context.Background(), `DELETE FROM workspaces WHERE id = $1`, ws); err != nil { + t.Errorf("cleanup: %v", err) + } + }) + + for _, name := range []string{"PurgeSends", "PurgeInboxThreads", "PurgeDeliverabilityEvents"} { + t.Run(name, func(t *testing.T) { + plan := genericPlan(t, ctx, pool, name) + t.Logf("generic plan for %s:\n%s", name, plan) + if strings.Contains(plan, "Anti Join") { + t.Errorf("%s's guards were pulled up into an anti-join; each must stay a per-candidate SubPlan (NOT EXISTS ... OFFSET 0)", name) + } + if !strings.Contains(plan, "SubPlan") { + t.Errorf("%s has no SubPlan: the guards are not being evaluated per candidate", name) + } + if !strings.Contains(plan, "Limit") { + t.Errorf("%s has no Limit on its candidate scan", name) + } + }) + } +} + +// genericPlan PREPAREs the query and EXPLAINs it under force_generic_plan, on one +// connection so the session settings apply. EXPLAIN without ANALYZE does not run +// the DELETEs. +func genericPlan(t *testing.T, ctx context.Context, pool *pgxpool.Pool, name string) string { + t.Helper() + sql, params := retentionQuery(t, name) + conn, err := pool.Acquire(ctx) + if err != nil { + t.Fatalf("acquire: %v", err) + } + defer conn.Release() + stmt := "plan_" + strings.ToLower(name) + for _, s := range []string{ + "SET plan_cache_mode = force_generic_plan", + "DEALLOCATE ALL", + "PREPARE " + stmt + " AS " + sql, + } { + if _, err := conn.Exec(ctx, s); err != nil { + t.Fatalf("%s: %v", strings.SplitN(s, " AS ", 2)[0], err) + } + } + args := make([]string, len(params)) + for i, p := range params { + switch p { + case "older_than_seconds": + args[i] = "34560000" // 400 days + case "after_at": + args[i] = "'0001-01-01T00:00:00Z'" + case "after_id": + args[i] = "'" + uuid.Nil.String() + "'" + case "batch_limit": + args[i] = "20000" + default: + t.Fatalf("unexpected parameter %q in %s", p, name) + } + } + rows, err := conn.Query(ctx, "EXPLAIN (COSTS OFF) EXECUTE "+stmt+"("+strings.Join(args, ", ")+")") + if err != nil { + t.Fatalf("explain: %v", err) + } + defer rows.Close() + var lines []string + for rows.Next() { + var line string + if err := rows.Scan(&line); err != nil { + t.Fatalf("scan plan: %v", err) + } + lines = append(lines, line) + } + if err := rows.Err(); err != nil { + t.Fatalf("plan: %v", err) + } + return strings.Join(lines, "\n") +} + +// seedRetentionVolume gives the planner a guard table worth hash-joining: tens of +// thousands of old sends, threads and enrollments in one workspace, then ANALYZE +// so the statistics reflect them. +func seedRetentionVolume(t *testing.T, ctx context.Context, pool *pgxpool.Pool) uuid.UUID { + t.Helper() + var ws uuid.UUID + steps := []string{ + `INSERT INTO workspaces (name) VALUES ('retention plan ' || gen_random_uuid()) RETURNING id`, + } + if err := pool.QueryRow(ctx, steps[0]).Scan(&ws); err != nil { + t.Fatalf("workspace: %v", err) + } + for _, s := range []string{ + `INSERT INTO mailboxes (workspace_id, email, secret_ciphertext) VALUES ($1, 'plan@x.test', 'x')`, + `INSERT INTO lists (workspace_id, name) VALUES ($1, 'L')`, + `INSERT INTO campaigns (workspace_id, name, mailbox_id, list_id, subject, status) + SELECT $1, 'C', m.id, l.id, 'S', 'done' FROM mailboxes m, lists l WHERE m.workspace_id = $1 AND l.workspace_id = $1`, + `INSERT INTO contacts (workspace_id, email) SELECT $1, 'p' || g || '@x.test' FROM generate_series(1, 30000) g`, + `INSERT INTO sends (workspace_id, campaign_id, contact_id, mailbox_id, to_email, status, created_at, sent_at, step_order) + SELECT $1, c.id, ct.id, c.mailbox_id, ct.email, 'sent', now() - interval '500 days', now() - interval '500 days', 1 + FROM campaigns c JOIN contacts ct ON ct.workspace_id = c.workspace_id WHERE c.workspace_id = $1`, + `INSERT INTO sequence_enrollments (workspace_id, campaign_id, contact_id, status) + SELECT $1, c.id, ct.id, 'completed' FROM campaigns c JOIN contacts ct ON ct.workspace_id = c.workspace_id WHERE c.workspace_id = $1`, + `INSERT INTO inbox_threads (workspace_id, mailbox_id, campaign_id, contact_id, last_message_at) + SELECT $1, c.mailbox_id, c.id, ct.id, now() - interval '500 days' + FROM campaigns c JOIN contacts ct ON ct.workspace_id = c.workspace_id WHERE c.workspace_id = $1`, + `INSERT INTO deliverability_events (workspace_id, kind, email, provider_event_id, received_at) + SELECT $1, 'bounce', 'p' || g || '@x.test', 'plan-' || g || '-' || gen_random_uuid(), now() - interval '500 days' + FROM generate_series(1, 30000) g`, + } { + if _, err := pool.Exec(ctx, s, ws); err != nil { + t.Fatalf("seed %q: %v", s[:40], err) + } + } + for _, table := range []string{"sends", "sequence_enrollments", "inbox_threads", "deliverability_events", "deals", "inbox_pending_replies", "inbox_thread_snoozes", "inbox_messages", "campaigns"} { + if _, err := pool.Exec(ctx, "ANALYZE "+table); err != nil { + t.Fatalf("analyze %s: %v", table, err) + } + } + return ws +} diff --git a/internal/platform/db/tenancyqueries_test.go b/internal/platform/db/tenancyqueries_test.go index e849ddf7..c3b6a812 100644 --- a/internal/platform/db/tenancyqueries_test.go +++ b/internal/platform/db/tenancyqueries_test.go @@ -135,6 +135,7 @@ var tenancyExceptions = map[string]string{ "maintenance.sql:PurgeDeadWorkers": "reaps the global worker registry and the assignments pinned to dead workers; workers are deployment infra, not tenant data (migration 000017).", "recipientdomain.sql:DeleteExpiredRecipientDomains": "retention sweep over a DNS-fact cache, by age alone. A lost row costs one re-lookup.", "warmup.sql:PurgeWarmupObservations": "retention sweep over append-only warmup evidence, by age alone, returning a count (design §4.6).", + "retention.sql:RollupTrackingEvents": "retention sweep folding raw tracking events past the operator's window into per-send rollups, by age alone, returning counts and a cursor. The rollup's workspace_id is copied from the deleted rows (the tenant FK to sends fixes it), never supplied by a caller.", "deadletter.sql:PurgeTaskDeadLetters": "retention sweep over captured retry-exhausted tasks, by age alone, returning a count. Same shape and same reasoning as PurgeWarmupObservations: the table is append-only in practice and had no sweep at all.", "webhook.sql:PurgeWebhookDeliveries": "retention sweep over the outbound-webhook delivery log, by age alone, returning a count. Same shape and reasoning as PurgeTaskDeadLetters: one row per (event, endpoint), append-only from the app, and no sweep of its own.", "fleet.sql:PurgeFleetDecisions": "retention sweep over the append-only fleet decision log, by age alone, returning a count. The table carries workspace_id because a decision naming a mailbox is tenant data, but the 90-day purge is deployment maintenance across every tenant — scoping it would leave any workspace the sweep did not name growing forever.", @@ -167,8 +168,21 @@ var createTable = regexp.MustCompile(`(?is)CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS // starts being guarded automatically instead of silently staying exempt. var addColumn = regexp.MustCompile(`(?is)ALTER\s+TABLE\s+(?:ONLY\s+)?"?([a-z0-9_]+)"?\s+ADD\s+COLUMN\s+(?:IF\s+NOT\s+EXISTS\s+)?"?([a-z0-9_]+)"?`) -// tenantScopedTables returns every table that has a workspace_id column, derived from -// the embedded up-migrations. Later migrations win, so a table dropped and recreated +// createView catches a view. A view that exposes workspace_id is tenant-scoped exactly +// as a table is: reading it without a workspace_id predicate returns every workspace's +// rows. Without this, moving a query from a table onto a view over that table would +// silently take it out of this guard — which is precisely what happened when the +// tracking aggregates moved onto tracking_engagement (20260923110315). +var createView = regexp.MustCompile(`(?is)CREATE\s+(?:OR\s+REPLACE\s+)?VIEW\s+"?([a-z0-9_]+)"?\s+AS\s+(.*?);`) + +// selectsTenantColumn reports whether a view body mentions workspace_id anywhere. Looser +// than declaresTenantColumn, and deliberately: a view's column list is a SELECT list, +// and any mention there means the view carries the tenant column. The bias is the +// guard's usual one — a false "tenant-scoped" costs one allowlist entry with a reason. +var tenantColumnWord = regexp.MustCompile(`(?i)\b` + tenantColumn + `\b`) + +// tenantScopedTables returns every table (and view) that has a workspace_id column, +// derived from the embedded up-migrations. Later migrations win, so a table dropped and recreated // (users, in 000004) is classified by its final shape. func tenantScopedTables(t *testing.T) map[string]bool { t.Helper() @@ -196,6 +210,9 @@ func tenantScopedTables(t *testing.T) map[string]bool { scoped[m[1]] = true } } + for _, m := range createView.FindAllStringSubmatch(sql, -1) { + scoped[m[1]] = tenantColumnWord.MatchString(m[2]) + } } return scoped } @@ -464,7 +481,7 @@ func TestEveryTenancyExceptionHasAWrittenReason(t *testing.T) { // this guard has stopped guarding, so the count is the size of the hole in the net. // Raising it should be a conscious act in a diff, not a drift. func TestTheTenancyAllowlistDoesNotGrowSilently(t *testing.T) { - const known = 49 + const known = 50 if got := len(tenancyExceptions); got != known { t.Errorf("tenancyExceptions has %d entries, expected %d. Every entry is a query this "+ "guard no longer checks. If you added one deliberately, update `known` in the same "+ diff --git a/internal/platform/db/trackingreaders_test.go b/internal/platform/db/trackingreaders_test.go new file mode 100644 index 00000000..54912f89 --- /dev/null +++ b/internal/platform/db/trackingreaders_test.go @@ -0,0 +1,125 @@ +package db + +import ( + "io/fs" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" +) + +// Reading tracking_events DIRECTLY silently loses every event older than the +// tracking retention window, because retention rolls those rows up into +// tracking_event_rollups and deletes them (migration 20260923110315). A reader +// that should see them — anything that counts, ranks, or decides on opens and +// clicks — must read the tracking_engagement view instead. +// +// The failure mode is the reason for a test rather than a comment. A query that +// reads the raw table passes every test in the repo, because no fixture has ever +// been rolled up; it only goes wrong in production, months after the operator +// enabled retention, as an open rate that fell or — for a sequence branch on +// "opened within N days" — as enrollments quietly taking the wrong branch and +// being sent the wrong email. +// +// So every reference to tracking_events in a query file or in Go SQL outside +// tests is listed here with its reason. A new one fails this test until someone +// either moves it onto tracking_engagement or adds it with a reason that +// explains why the rolled-up rows genuinely do not matter to it. +var rawTrackingReaders = map[string]string{ + // queries/*.sql, keyed file:Query. + "tracking.sql:InsertTrackingEvent": "the writer.", + "tracking.sql:CountRecentSendOpensFromSubnet": "the bot classifier's burst rule: needs client_ip (the rollup drops it) and looks back " + + "botfilter.BurstWindow (10 minutes), far inside the shortest tracking window retention accepts (30 days).", + "retention.sql:RollupTrackingEvents": "retention itself: the statement that moves raw rows into the rollup.", + "retention.sql:PurgeSends": "retention itself: deletes a deleted send's raw events.", + // Go SQL outside queries/, keyed by repo-relative path. + "internal/app/crm/integration_store.go": "the CRM deal activity feed (ListEvents) lists individual open/click EVENTS; a rollup cannot " + + "preserve an event log, so rolled-up events leave the feed by design (documented in the deploy docs).", + "internal/sandbox/store.go": "the sandbox seeder WRITES fixture events, and its NOT EXISTS guards its own re-run; it reads nothing reported.", +} + +var rawTrackingRef = regexp.MustCompile(`(?i)\b(?:FROM|JOIN|INTO|UPDATE)\s+(?:ONLY\s+)?tracking_events\b`) + +func TestNothingReadsRawTrackingEventsOutsideTheAllowlist(t *testing.T) { + found := map[string]bool{} + + for _, q := range namedQueries(t) { + if rawTrackingRef.MatchString(stripComments(q.body)) { + found[q.file+":"+q.name] = true + } + } + + root := filepath.Join("..", "..", "..") + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + switch d.Name() { + case "gen", "web", "node_modules", ".git", "docs", ".claude": + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + src, err := os.ReadFile(path) + if err != nil { + return err + } + if rawTrackingRef.MatchString(stripGoComments(string(src))) { + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + found[filepath.ToSlash(rel)] = true + } + return nil + }) + if err != nil { + t.Fatalf("walk source tree: %v", err) + } + if len(found) == 0 { + t.Fatal("found no reference to tracking_events at all — the scan is not reading what it thinks it is, so it would pass vacuously") + } + + var unexpected, stale []string + for ref := range found { + if _, ok := rawTrackingReaders[ref]; !ok { + unexpected = append(unexpected, ref) + } + } + for ref := range rawTrackingReaders { + if !found[ref] { + stale = append(stale, ref) + } + } + sort.Strings(unexpected) + sort.Strings(stale) + for _, ref := range unexpected { + t.Errorf("%s reads tracking_events directly. Once tracking retention is enabled that table no longer holds "+ + "events older than the window — read tracking_engagement (raw + rolled up) instead, or, if the rolled-up "+ + "rows genuinely do not matter here, add it to rawTrackingReaders with the reason.", ref) + } + for _, ref := range stale { + t.Errorf("rawTrackingReaders lists %s, which no longer reads tracking_events — remove the entry", ref) + } +} + +// stripGoComments drops // line comments, so prose that mentions the table (as +// this repository's comments often do) is not mistaken for a query. SQL lives in +// string literals, which this leaves alone. +func stripGoComments(src string) string { + var b strings.Builder + for _, line := range strings.Split(src, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "//") { + continue + } + b.WriteString(line) + b.WriteByte('\n') + } + return b.String() +} diff --git a/internal/platform/jobrun/jobrun.go b/internal/platform/jobrun/jobrun.go index 38f4deec..51355fb9 100644 --- a/internal/platform/jobrun/jobrun.go +++ b/internal/platform/jobrun/jobrun.go @@ -3,12 +3,12 @@ // scheduled_job_runs instead of a log grep. // // internal/platform/metrics.SweepCompleted already exists, but only four of -// the eight periodic reconciles in cmd/worker/scheduler.go's sweepRegistrars() +// the nine periodic reconciles in cmd/worker/scheduler.go's sweepRegistrars() // call it (the enrollment, inbox, warmup and stranded-pending-send scans), and // even where it is called a Prometheus counter does not survive a scrape gap // and carries no error message — exactly what an operator needs when a sweep // silently stops running, or starts failing every tick. Record is a decorator -// that answers both gaps for all eight, uniformly, WITHOUT changing any handler +// that answers both gaps for all nine, uniformly, WITHOUT changing any handler // body. // // This package knows nothing about Postgres. Record takes a Recorder @@ -52,6 +52,7 @@ const ( NameInboxSweep = "inbox sweep" NameWarmupSweep = "warmup sweep" NameMaintenanceCleanup = "maintenance cleanup" + NameRetention = "retention" NameDomainAuthSweep = "domain auth sweep" NameRecipientESPSweep = "recipient esp sweep" NameFleetRotate = "fleet rotation" @@ -93,7 +94,7 @@ const truncatedErrorMarker = " …[truncated]" // // What this is NOT. The column's own comment used to say the value is "never // tenant content", and this decorator cannot promise that: it stores -// err.Error() from eight handlers it does not own, and any one of them wrapping a +// err.Error() from nine handlers it does not own, and any one of them wrapping a // mailbox address, a recipient or a subject line would make the claim false the // day it was written. What CAN be guaranteed is a bound, so that is what is // guaranteed — an unbounded column reachable by arbitrary error text, in a table diff --git a/internal/platform/metrics/exposition_test.go b/internal/platform/metrics/exposition_test.go index a72af403..5ae98943 100644 --- a/internal/platform/metrics/exposition_test.go +++ b/internal/platform/metrics/exposition_test.go @@ -41,6 +41,7 @@ func TestExpositionListsEveryInroadSeries(t *testing.T) { m.JobRunCompleted("domain auth sweep", "ok", time.Second) m.WorkerAssignmentStale() m.FleetRotated("unhealthy") + m.RetentionRowsDeleted("sends", 1, 1) if err := m.RegisterPool(realPoolStat(t)); err != nil { t.Fatalf("register pool: %v", err) } @@ -80,6 +81,7 @@ func TestExpositionListsEveryInroadSeries(t *testing.T) { "inroad_http_requests_total", "inroad_job_run_seconds", "inroad_queue_depth", + "inroad_retention_rows_deleted_total", "inroad_send_claims_total", "inroad_sends_total", "inroad_sweep_rows_total", diff --git a/internal/platform/metrics/metrics.go b/internal/platform/metrics/metrics.go index 0a7e91dd..5a87091f 100644 --- a/internal/platform/metrics/metrics.go +++ b/internal/platform/metrics/metrics.go @@ -65,6 +65,7 @@ type Metrics struct { jobRunSeconds *prometheus.HistogramVec workerAssignmentStale prometheus.Counter fleetRotations *prometheus.CounterVec + retentionRows *prometheus.CounterVec } // sweepRowBuckets bound the rows-scanned histogram-free counter's companion @@ -132,10 +133,14 @@ func New() *Metrics { Name: "inroad_fleet_rotations_total", Help: `Mailboxes the rotation pass actually MOVED to a different worker, labeled by the tier that justified each move ("unreachable"|"unhealthy"|"balance"). A rising "unreachable" is a host dying, "unhealthy" is a provider refusing a worker's egress IP, and "balance" is opportunistic repacking — which should be near-flat, since a mailbox that moves discards the IP trust it had accrued.`, }, []string{"tier"}), + retentionRows: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "inroad_retention_rows_deleted_total", + Help: `Rows the retention sweep (maintenance:retention) deleted, labeled by table ("deliverability_events"|"inbox_threads"|"tracking_events"|"sends"|"task_dead_letters") and scope ("table" for the table's own rows, "dependent" for rows removed with them: an inbox thread's messages, a send's tracking events and rollups). Flat while a window is enabled means nothing is aging out; a table that never drains logs "retention backlog not drained".`, + }, []string{"table", "scope"}), } m.registry.MustRegister( m.httpRequests, m.httpDuration, m.sends, m.claims, m.sweepDuration, m.sweepRows, m.jobRunSeconds, - m.workerAssignmentStale, m.fleetRotations, + m.workerAssignmentStale, m.fleetRotations, m.retentionRows, collectors.NewGoCollector(), collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}), ) @@ -259,8 +264,8 @@ func (m *Metrics) SweepCompleted(kind string, rows int, elapsed time.Duration) { // // Deliberately separate from SweepCompleted: that metric carries a row count // SweepCompleted's four callers (the enrollment, inbox, warmup and stranded- -// pending-send sweeps) already have in hand, and the other four wrapped jobs -// (maintenance cleanup, domain auth sweep, recipient esp sweep, fleet +// pending-send sweeps) already have in hand, and the other five wrapped jobs +// (maintenance cleanup, retention, domain auth sweep, recipient esp sweep, fleet // rotation) do not report one to the decorator — inventing rows=0 for them // here would put fabricated data in SweepCompleted's series. This metric // reports only what jobrun.Record itself observes: wall time and outcome, @@ -409,3 +414,24 @@ func (rec *statusRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) { } return hj.Hijack() } + +// RetentionRowsDeleted records one table's share of a retention run: rows of the +// table itself, and rows removed along with them. table is one of the sweep's +// fixed table names (internal/worker/maintenance.retentionTables), never a value +// derived from data, so the label set is bounded by construction. +// +// Zero counts are not added — a Prometheus counter only goes up, and a disabled +// or empty table should not mint a series it never moves. +// +// A nil receiver is a no-op, like every other method on Metrics. +func (m *Metrics) RetentionRowsDeleted(table string, rows, dependents int64) { + if m == nil { + return + } + if rows > 0 { + m.retentionRows.WithLabelValues(table, "table").Add(float64(rows)) + } + if dependents > 0 { + m.retentionRows.WithLabelValues(table, "dependent").Add(float64(dependents)) + } +} diff --git a/internal/platform/metrics/metrics_test.go b/internal/platform/metrics/metrics_test.go index 32315727..a5197d86 100644 --- a/internal/platform/metrics/metrics_test.go +++ b/internal/platform/metrics/metrics_test.go @@ -217,6 +217,36 @@ func TestFleetRotatedSeparatesTheTiers(t *testing.T) { } } +// TestRetentionRowsDeletedSeparatesTableFromDependents pins the scope label. A +// deleted inbox thread takes its messages with it and a deleted send takes its +// tracking events: folded into one number, "the sweep deleted 5,000 threads" and +// "the sweep deleted 5,000 messages" would be indistinguishable, and the second +// is a hundredth of the first. +func TestRetentionRowsDeletedSeparatesTableFromDependents(t *testing.T) { + m := metrics.New() + m.RetentionRowsDeleted("inbox_threads", 3, 40) + m.RetentionRowsDeleted("inbox_threads", 2, 0) + // A disabled or empty table reports zeros; they must not mint a series. + m.RetentionRowsDeleted("sends", 0, 0) + + families := metricstest.Scrape(t, m) + if got := metricstest.CounterValue(families, "inroad_retention_rows_deleted_total", + map[string]string{"table": "inbox_threads", "scope": "table"}); got != 5 { + t.Errorf("inbox_threads{scope=table} = %v, want 5", got) + } + if got := metricstest.CounterValue(families, "inroad_retention_rows_deleted_total", + map[string]string{"table": "inbox_threads", "scope": "dependent"}); got != 40 { + t.Errorf("inbox_threads{scope=dependent} = %v, want 40", got) + } + for _, sample := range families["inroad_retention_rows_deleted_total"].GetMetric() { + for _, lp := range sample.GetLabel() { + if lp.GetName() == "table" && lp.GetValue() == "sends" { + t.Errorf("a zero count minted a sends series") + } + } + } +} + // TestNewRegistersRuntimeAndProcessCollectors pins the free-and-standard // collectors being present on every process's registry without any wiring. func TestNewRegistersRuntimeAndProcessCollectors(t *testing.T) { @@ -240,6 +270,7 @@ func TestNoTenantLabelsAnywhere(t *testing.T) { m.SendClaimed("step", metrics.ClaimOutcomeWon) m.SweepCompleted("inbox", 1, time.Second) m.FleetRotated("unhealthy") + m.RetentionRowsDeleted("sends", 1, 1) banned := map[string]bool{ "workspace_id": true, "workspace": true, "tenant_id": true, "tenant": true, @@ -270,6 +301,7 @@ func TestNilMetricsNoOps(t *testing.T) { m.SendClaimed("step", metrics.ClaimOutcomeWon) m.SweepCompleted("inbox", 12, 3*time.Second) m.FleetRotated("unreachable") + m.RetentionRowsDeleted("sends", 3, 4) r := chi.NewRouter() r.Use(m.HTTPMiddleware()) diff --git a/internal/platform/queue/queue.go b/internal/platform/queue/queue.go index c32eb390..6d9042a9 100644 --- a/internal/platform/queue/queue.go +++ b/internal/platform/queue/queue.go @@ -157,6 +157,7 @@ var taskQueues = map[string]string{ TaskInboxSweep: QueueControl, TaskSweepEnrollments: QueueControl, TaskMaintenanceCleanup: QueueControl, + TaskMaintenanceRetention: QueueControl, TaskDomainAuthSweep: QueueControl, TaskRecipientESPSweep: QueueControl, TaskFleetRotate: QueueControl, @@ -249,6 +250,15 @@ const TaskWarmupSweep = "warmup:sweep" // artifacts (sessions, challenges, one-time codes, and OAuth credentials). const TaskMaintenanceCleanup = "maintenance:cleanup" +// TaskMaintenanceRetention is the hourly, operator-configured retention sweep +// over the recipient-identifying tables (sends, tracking events, deliverability +// events, inbox threads) and captured dead letters — INROAD_RETENTION_*_DAYS. +// Separate from maintenance:cleanup because its windows are the operator's +// rather than fixed by the code, and because it runs in bounded batches often +// enough to keep up with the busiest tables in the schema, where the daily +// cleanup purges tables that grow slowly. +const TaskMaintenanceRetention = "maintenance:retention" + // TaskDomainAuthSweep re-checks the SPF/DKIM/DMARC records of every sending // domain whose last completed check is older than the staleness window. const TaskDomainAuthSweep = "domainauth:sweep" @@ -1138,6 +1148,16 @@ func RegisterMaintenanceCleanup(sch *asynq.Scheduler) error { return registerControlSweep(sch, "@every 24h", TaskMaintenanceCleanup) } +// RegisterMaintenanceRetention registers the recipient-data retention sweep. +// Hourly rather than daily: each run is capped (internal/worker/maintenance +// RetentionOptions) so no single run holds a worker slot for long, and the cap +// times 24 runs is what lets the sweep keep pace with sends and tracking_events, +// which can grow by millions of rows a day. A tick with every table disabled or +// nothing aged out costs one indexed probe per enabled table. +func RegisterMaintenanceRetention(sch *asynq.Scheduler) error { + return registerControlSweep(sch, "@every 1h", TaskMaintenanceRetention) +} + // RegisterDomainAuthSweep registers the periodic domain-authentication sweep. // It ticks hourly against a 24-hour staleness window: the tick rate is what // bounds how long a NEWLY connected mailbox's domain sits unchecked, and how diff --git a/internal/platform/queue/queue_test.go b/internal/platform/queue/queue_test.go index 4d848234..f7c23a51 100644 --- a/internal/platform/queue/queue_test.go +++ b/internal/platform/queue/queue_test.go @@ -106,6 +106,7 @@ func TestQueueForTaskTypeIsTheOneRoutingTable(t *testing.T) { TaskInboxSweep: QueueControl, TaskSweepEnrollments: QueueControl, TaskMaintenanceCleanup: QueueControl, + TaskMaintenanceRetention: QueueControl, TaskDomainAuthSweep: QueueControl, TaskRecipientESPSweep: QueueControl, TaskDeliverabilityEvaluate: QueueControl, @@ -337,6 +338,7 @@ func TestEveryControlSweepTargetsControlQueue(t *testing.T) { {"inbox sweep", "@every " + inboxSweepInterval.String(), TaskInboxSweep}, {"warmup sweep", "@every 5m", TaskWarmupSweep}, {"maintenance cleanup", "@every 24h", TaskMaintenanceCleanup}, + {"maintenance retention", "@every 1h", TaskMaintenanceRetention}, {"domain auth sweep", "@every 1h", TaskDomainAuthSweep}, {"recipient esp sweep", "@every 5m", TaskRecipientESPSweep}, {"fleet rotate", "@every 5m", TaskFleetRotate}, diff --git a/internal/worker/capabilities_test.go b/internal/worker/capabilities_test.go index df0ccd8c..b7b1c546 100644 --- a/internal/worker/capabilities_test.go +++ b/internal/worker/capabilities_test.go @@ -21,8 +21,8 @@ import ( // // # What this exists to catch // -// Eighteen capabilities are reached by COMMA-OK TYPE ASSERTION on whatever -// value the composition root put in Deps.Core — six in handlers.go, three in +// Nineteen capabilities are reached by COMMA-OK TYPE ASSERTION on whatever +// value the composition root put in Deps.Core — seven in handlers.go, three in // inbox.RegisterPerMessage, six feature-detected per message inside // PollHandler, and three in cmd/worker. A type assertion is invisible to the // compiler. When one stops matching, nothing fails to build: the registrar @@ -146,6 +146,8 @@ func TestBothCoreAPIImplementationsCarryEveryOptionalCapability(t *testing.T) { assertsAs[deliverability.Breaker](impl.core)}, {"maintenance.Cleaner", "maintenance:cleanup is never registered; nothing is ever purged", assertsAs[maintenance.Cleaner](impl.core)}, + {"maintenance.Retainer", "maintenance:retention is never registered; no configured retention window is ever applied", + assertsAs[maintenance.Retainer](impl.core)}, {"recipientesp.Core", "the recipient-ESP cache is never refreshed; sender matching degrades to unmatched", assertsAs[recipientesp.Core](impl.core)}, {"fleet.Rotator", "a mailbox on a provider-blocked worker is never moved off it", diff --git a/internal/worker/handlers.go b/internal/worker/handlers.go index 531d2eaa..cd207d6a 100644 --- a/internal/worker/handlers.go +++ b/internal/worker/handlers.go @@ -79,10 +79,19 @@ type Deps struct { WebhookAllowPrivate bool + // Retention is how long the retention sweep (maintenance:retention) keeps + // each recipient-identifying table and the dead-letter capture, built by the + // composition root from INROAD_RETENTION_*_DAYS and already validated there. + // Its zero value disables every table INCLUDING dead letters, so a Deps + // literal that omits it runs a sweep that deletes nothing — the safe + // direction for a destructive job, and cmd/worker's own test pins that the + // real composition root does set it. + Retention maintenance.RetentionPolicy + // Metrics records inroad_sends_total at the campaign and warmup send // handlers' finalize points, inroad_sweep_seconds / inroad_sweep_rows_total // at the enrollment, inbox, warmup and stranded-pending-send sweeps, and - // (via jobrun.Record below) inroad_job_run_seconds at all eight of the + // (via jobrun.Record below) inroad_job_run_seconds at all nine of the // periodic reconciles in cmd/worker/scheduler.go's sweepRegistrars(); a nil // Metrics (metrics disabled) no-ops throughout. Metrics *metrics.Metrics @@ -114,7 +123,7 @@ func Register(mux *asynq.ServeMux, d Deps) { } } -// registerScheduled wires the eight periodic reconciles and the campaign breaker. +// registerScheduled wires the nine periodic reconciles and the campaign breaker. // These scan or delete ACROSS TENANTS, so they are registered on the control // role only: a send host runs no handler that enumerates every workspace's due // enrollments (fleet design §1.2). @@ -129,13 +138,23 @@ func registerScheduled(mux *asynq.ServeMux, d Deps, recorder jobrun.Recorder) { if cleaner, ok := d.Core.(maintenance.Cleaner); ok { mux.HandleFunc(queue.TaskMaintenanceCleanup, jobrun.Record(recorder, d.Metrics, jobrun.NameMaintenanceCleanup, maintenance.CleanupHandler(cleaner))) } + // Recipient-data retention: the operator-configured windows over sends, + // tracking events, deliverability events, inbox threads and dead letters. + // Same comma-ok as the cleaner, and the absence is the point for one of the + // two implementations: *remote.Client does not satisfy Retainer, because + // every batch is a cross-tenant delete (coreapi/retention.go), so a fleet + // host is never handed this sweep even if its role were misconfigured. + if retainer, ok := d.Core.(maintenance.Retainer); ok { + mux.HandleFunc(queue.TaskMaintenanceRetention, jobrun.Record(recorder, d.Metrics, jobrun.NameRetention, + maintenance.RetentionHandler(retainer, d.Retention, d.Metrics, maintenance.RetentionOptions{}))) + } // Campaign circuit breaker. Registered by type assertion for the same reason // as the cleaner above: the capability is consumed through a one-method // interface rather than widening coreapi.Client (and its 13 test fakes) to // carry it. A Client that does not implement it simply has no breaker, which // is what a future HTTP coreapi would report until it grows the endpoint. // - // Not one of the eight jobrun.Record wraps: deliverability:evaluate is not in + // Not one of the nine jobrun.Record wraps: deliverability:evaluate is not in // cmd/worker/scheduler.go's sweepRegistrars() (it fires per-send-batch, not // on a fixed schedule), so it is outside this ledger's scope. It is still // control-plane work despite that: it reads campaign-wide aggregates and @@ -168,7 +187,7 @@ func registerScheduled(mux *asynq.ServeMux, d Deps, recorder jobrun.Recorder) { mux.HandleFunc(queue.TaskFleetRotate, jobrun.Record(recorder, d.Metrics, jobrun.NameFleetRotate, fleet.RotateHandler(fr))) } - // Warmup: the fan-out/health sweep. Only warmup:sweep is one of the eight + // Warmup: the fan-out/health sweep. Only warmup:sweep is one of the nine // periodic reconciles; warmup:tick and warmup:engage are per-message // follow-ups, not scheduled sweeps, so they are outside the ledger the same // way warmup send/finalize is (see registerPerMessage). diff --git a/internal/worker/inbox/register.go b/internal/worker/inbox/register.go index 9e46da75..dc66812f 100644 --- a/internal/worker/inbox/register.go +++ b/internal/worker/inbox/register.go @@ -19,7 +19,7 @@ import ( // task will deliver. Both enumerate rows across every workspace, so they belong // to the control role only (see internal/worker/role.go). mtx records each // sweep's duration and candidate count; a nil mtx no-ops. recorder is the same -// jobrun.Recorder handlers.go resolved once for all eight periodic reconciles +// jobrun.Recorder handlers.go resolved once for all nine periodic reconciles // (nil when the coreapi client doesn't implement it); these two are the // scheduled reconciles in this package (see cmd/worker/scheduler.go's // sweepRegistrars()), so they alone are wrapped in jobrun.Record — the diff --git a/internal/worker/maintenance/cleanup.go b/internal/worker/maintenance/cleanup.go index 69a9d751..3b68b727 100644 --- a/internal/worker/maintenance/cleanup.go +++ b/internal/worker/maintenance/cleanup.go @@ -27,19 +27,15 @@ type Cleaner interface { // window plus the mailbox assignments pinned to them. Separate again: this is // global infrastructure state, not a security artifact or an HTTP concern. PurgeDeadWorkers(ctx context.Context) (deleted int64, err error) - // PurgeDeadLetters removes captured retry-exhausted tasks past their 90-day - // retention. Separate for the same reason as the two above: a dead letter is - // a record of dropped work, neither a security artifact nor an HTTP concern. - // It is here at all because the table had no sweep and grows with failures - // nobody schedules — the reasoning behind invariant 55's warmup purge. - PurgeDeadLetters(ctx context.Context) (deleted int64, err error) // PurgeWebhookDeliveries removes webhook_deliveries rows past their 30-day - // retention. Same reasoning as PurgeDeadLetters: append-only in practice, - // grows one row per (event, endpoint), and had no sweep of its own. + // retention. Append-only in practice, grows one row per (event, endpoint), + // and had no sweep of its own — the reasoning behind invariant 55's warmup + // purge. (Captured dead letters used to be purged here too, on a hard-coded + // 90 days; they moved to the configurable retention sweep, retention.go.) PurgeWebhookDeliveries(ctx context.Context) (deleted int64, err error) // PurgeScheduledJobRuns removes scheduled_job_runs rows past their 30-day - // retention. Same reasoning as PurgeDeadLetters/PurgeWebhookDeliveries: - // append-only from internal/platform/jobrun.Record, eight jobs writing a row + // retention. Same reasoning as PurgeWebhookDeliveries: + // append-only from internal/platform/jobrun.Record, nine jobs writing a row // per run (several every five minutes), and no sweep of its own until this // one — see the table's migration for why it needed one from day one rather // than growing unbounded first. @@ -60,8 +56,8 @@ type Cleaner interface { // CleanupHandler purges, in order: expired security artifacts, expired // Idempotency-Key replay-cache rows, warmup evidence past its retention window, -// dead workers with their mailbox assignments, captured dead letters past -// theirs, expired webhook deliveries, scheduled-job-run ledger rows past theirs, +// dead workers with their mailbox assignments, expired webhook deliveries, +// scheduled-job-run ledger rows past theirs, // per-worker provider signal windows, and fleet decision-log rows. Returning a // database error from any purge lets asynq retry; successful runs log each // affected count for observability. @@ -91,12 +87,6 @@ func CleanupHandler(core Cleaner) func(context.Context, *asynq.Task) error { } slog.InfoContext(ctx, "dead workers and their assignments purged", "rows", workersDeleted) - deadLettersDeleted, err := core.PurgeDeadLetters(ctx) - if err != nil { - return err - } - slog.InfoContext(ctx, "expired dead letters purged", "rows", deadLettersDeleted) - webhookDeliveriesDeleted, err := core.PurgeWebhookDeliveries(ctx) if err != nil { return err diff --git a/internal/worker/maintenance/cleanup_test.go b/internal/worker/maintenance/cleanup_test.go index 7e55bfe1..249f58ac 100644 --- a/internal/worker/maintenance/cleanup_test.go +++ b/internal/worker/maintenance/cleanup_test.go @@ -24,8 +24,6 @@ type cleanupCore struct { observationsErr error workersDeleted int64 workersErr error - deadLettersDeleted int64 - deadLettersErr error webhooksDeleted int64 webhooksErr error jobRunsDeleted int64 @@ -38,7 +36,6 @@ type cleanupCore struct { idempotencyCalled bool observationsCalled bool workersCalled bool - deadLettersCalled bool webhooksCalled bool jobRunsCalled bool signalsCalled bool @@ -65,11 +62,6 @@ func (c *cleanupCore) PurgeDeadWorkers(context.Context) (int64, error) { return c.workersDeleted, c.workersErr } -func (c *cleanupCore) PurgeDeadLetters(context.Context) (int64, error) { - c.deadLettersCalled = true - return c.deadLettersDeleted, c.deadLettersErr -} - func (c *cleanupCore) PurgeWebhookDeliveries(context.Context) (int64, error) { c.webhooksCalled = true return c.webhooksDeleted, c.webhooksErr @@ -91,14 +83,14 @@ func (c *cleanupCore) PurgeFleetDecisions(context.Context) (int64, error) { } func TestCleanupHandler(t *testing.T) { - // Nine DISTINCT counts. The log line is the only observable this job has — + // Eight DISTINCT counts. The log line is the only observable this job has — // nothing returns the numbers — so the assertion below is what makes the // fixture values mean anything, and distinct values are what turn "a count was // logged" into "the RIGHT count was logged": identical numbers would pass a // handler that logged the same variable every time. core := &cleanupCore{ deleted: 12, idempotencyDeleted: 3, observationsDeleted: 7, workersDeleted: 2, - deadLettersDeleted: 4, webhooksDeleted: 9, jobRunsDeleted: 6, + webhooksDeleted: 9, jobRunsDeleted: 6, signalsDeleted: 11, decisionsDeleted: 5, } @@ -122,9 +114,6 @@ func TestCleanupHandler(t *testing.T) { if !core.workersCalled { t.Fatal("PurgeDeadWorkers was not called") } - if !core.deadLettersCalled { - t.Fatal("PurgeDeadLetters was not called") - } if !core.webhooksCalled { t.Fatal("PurgeWebhookDeliveries was not called") } @@ -146,7 +135,6 @@ func TestCleanupHandler(t *testing.T) { {"expired idempotency keys purged", 3}, {"expired warmup observations purged", 7}, {"dead workers and their assignments purged", 2}, - {"expired dead letters purged", 4}, {"expired webhook deliveries purged", 9}, {"expired scheduled job runs purged", 6}, {"expired worker provider signals purged", 11}, @@ -178,21 +166,6 @@ func loggedRows(t *testing.T, logs []byte, msg string) int64 { return -1 } -// task_dead_letters had no retention sweep before this and grows with failures -// nobody schedules, so a purge that silently stopped running would be invisible -// until the table was a problem. The failure surfaces for retry instead. -func TestCleanupHandlerReturnsErrorOnDeadLetterPurgeFailure(t *testing.T) { - want := errors.New("db unavailable") - core := &cleanupCore{deadLettersErr: want} - err := CleanupHandler(core)(context.Background(), asynq.NewTask(queue.TaskMaintenanceCleanup, nil)) - if !errors.Is(err, want) { - t.Fatalf("handler error = %v, want %v", err, want) - } - if !core.workersCalled { - t.Fatal("the earlier purges should still have run") - } -} - // A dead worker's rows are inert for routing (the assigner's liveness join // already ignores them) but they keep inflating that worker's count in the // least-loaded pick, so a failed reap has to surface for retry rather than @@ -231,9 +204,6 @@ func TestCleanupHandlerReturnsErrorForRetry(t *testing.T) { if core.observationsCalled { t.Fatal("PurgeWarmupObservations must not run when CleanupExpired already failed") } - if core.deadLettersCalled { - t.Fatal("PurgeDeadLetters must not run when CleanupExpired already failed") - } } func TestCleanupHandlerReturnsErrorForRetryOnIdempotencyPurgeFailure(t *testing.T) { @@ -249,7 +219,7 @@ func TestCleanupHandlerReturnsErrorForRetryOnIdempotencyPurgeFailure(t *testing. } // scheduled_job_runs is the newest addition to this purge chain, run last. -// Like the dead-letter and dead-worker purges before it, its own failure must +// Like the dead-worker purge before it, its own failure must // surface for retry rather than let the run ledger grow unbounded in silence // (invariant 55's reasoning, applied to internal/platform/jobrun's writes). func TestCleanupHandlerReturnsErrorOnScheduledJobRunPurgeFailure(t *testing.T) { diff --git a/internal/worker/maintenance/retention.go b/internal/worker/maintenance/retention.go new file mode 100644 index 00000000..77ee8424 --- /dev/null +++ b/internal/worker/maintenance/retention.go @@ -0,0 +1,400 @@ +package maintenance + +import ( + "context" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/hibiken/asynq" + + "github.com/inroad/inroad/internal/coreapi" + "github.com/inroad/inroad/internal/platform/metrics" +) + +// Retainer is the narrow capability the retention sweep needs: one bounded batch +// per table. Separate from Cleaner because the two answer different questions — +// Cleaner purges things with a FIXED lifetime the code knows (a session expires, +// a replay-cache key is 24h), this purges records whose lifetime is the +// OPERATOR'S decision — and because each method here takes the window and the +// batch as arguments where Cleaner's take none. +// +// Satisfied by the in-process coreapi client only. The remote transport does not +// implement it, by design: every method is a cross-tenant DELETE (see +// coreapi/retention.go), so a fleet host can never be handed this sweep. +// +// What each batch keeps, and why, is documented on its SQL +// (internal/platform/db/queries/retention.sql): that is where the guards are, so +// that is where their reasons live. +type Retainer interface { + // PurgeDeliverabilityEvents deletes provider bounce/complaint events that no + // auto-pause rate can still read. + PurgeDeliverabilityEvents(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) + // PurgeInboxThreads deletes whole idle conversations, messages and all. + PurgeInboxThreads(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) + // RollupTrackingEvents folds raw open/click events into per-send rollups, + // then deletes them, so reporting survives and the per-hit detail does not. + RollupTrackingEvents(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) + // PurgeSends deletes campaign sends nothing live depends on. + PurgeSends(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) + // PurgeDeadLetters deletes captured retry-exhausted tasks, every status. + PurgeDeadLetters(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) + + // LoadRetentionProgress and SaveRetentionProgress persist each table's + // cursor across runs. A run that restarted from the oldest row every hour + // would spend its whole batch budget re-reading rows a guard keeps (the + // sends of long-lived active enrollments) and never reach the deletable + // rows behind them. + LoadRetentionProgress(ctx context.Context, table string) (coreapi.RetentionProgress, error) + SaveRetentionProgress(ctx context.Context, table string, cursor coreapi.RetentionCursor, newCycle bool) error + + // TryRetentionSweepLock makes the sweep single-instance deployment-wide. + // asynq elects no scheduler leader, so replicas can overlap a run; row-level + // SKIP LOCKED keeps that safe for rows but not across tables — the tracking + // rollup and the sends purge lock the same rows in opposite orders — and a + // shared cursor only means anything with one writer. release must be called + // once when acquired is true. + TryRetentionSweepLock(ctx context.Context) (release func() error, acquired bool, err error) +} + +// RetentionPolicy is how long each table keeps its rows. Zero DISABLES that +// table's sweep — it is never read as "keep nothing" (the in-process client +// refuses a non-positive window as a backstop, see encodeRetention). +// +// Built once, at the composition root, from INROAD_RETENTION_*_DAYS (see +// RetentionPolicyFromDays). The four recipient-data windows (sends; inbox +// threads with their messages; tracking events; deliverability events) default +// to disabled: how long to keep data about the people a workspace emails is a +// Privacy/Legal decision, not a code default. Dead letters default to 90 days +// (config.DefaultRetentionDeadLettersDays). +type RetentionPolicy struct { + DeliverabilityEvents time.Duration + InboxThreads time.Duration + TrackingEvents time.Duration + Sends time.Duration + DeadLetters time.Duration +} + +const day = 24 * time.Hour + +// retentionTable is one table's row in the sweep: what it is called, which +// setting feeds it, the shortest window it accepts and why, and its batch. +// +// ORDER IS LOAD-BEARING. A send is kept while a deliverability event names it or +// an inbox thread renders it, so those two tables run first and free their sends +// in the same run; tracking events run before sends so rows past the tracking +// window are rolled up before a send delete would cascade them away unrolled. +type retentionTable struct { + name string + setting string + floor time.Duration + why string + // scan is the rows one batch examines. Larger for the guarded tables, where + // most scanned rows may be kept and cost only an index probe each; smaller + // where every scanned row is deleted (and, for tracking, rolled up). + scan int32 + window func(RetentionPolicy) time.Duration + batch func(Retainer) func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) +} + +const ( + // guardedScan is the candidate bound for the tables whose rows a guard may + // keep. 20000 index-range rows plus a handful of index probes each is well + // inside the per-batch statement timeout (inprocess.retentionStatementTimeout). + guardedScan = 20000 + // unguardedScan matches every other purge in this repo (queries/maintenance.sql): + // every row scanned is deleted, and 5000 deletes is a sub-second statement. + unguardedScan = 5000 +) + +// retentionTables is the sweep, in the order it runs. +func retentionTables() []retentionTable { + return []retentionTable{ + { + name: "deliverability_events", setting: "INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS", floor: 90 * day, scan: guardedScan, + why: "the widest fixed rate window that reads these events is warmup health's 30 days (the breaker's own window is 7), " + + "and the dedup key goes with a deleted row, so a provider replay must be long past; 90 days is 3x the widest window, " + + "the same margin invariant 55 uses. A running campaign's unbounded fallback window is protected by a guard, not the floor", + window: func(p RetentionPolicy) time.Duration { return p.DeliverabilityEvents }, + batch: func(r Retainer) func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return r.PurgeDeliverabilityEvents + }, + }, + { + name: "inbox_threads", setting: "INROAD_RETENTION_INBOX_DAYS", floor: 30 * day, scan: guardedScan, + why: "the inbox's widest time scope is \"this week\" and a daily send cap reads today's outbound messages; " + + "30 days keeps a month of conversation an operator is plausibly still working", + window: func(p RetentionPolicy) time.Duration { return p.InboxThreads }, + batch: func(r Retainer) func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return r.PurgeInboxThreads + }, + }, + { + name: "tracking_events", setting: "INROAD_RETENTION_TRACKING_EVENTS_DAYS", floor: 30 * day, scan: unguardedScan, + why: "every reader of tracking events must read tracking_engagement, which includes rolled-up rows, and the few that read " + + "the raw table are pinned by trackingreaders_test.go (the bot classifier's 10-minute burst rule is the only one on a " + + "live decision path); 30 days leaves a month of per-hit detail for diagnosing a classification", + window: func(p RetentionPolicy) time.Duration { return p.TrackingEvents }, + batch: func(r Retainer) func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return r.RollupTrackingEvents + }, + }, + { + name: "sends", setting: "INROAD_RETENTION_SENDS_DAYS", floor: 90 * day, scan: guardedScan, + why: "warmup health counts 30 days of sends as its denominator, a late bounce or reply is matched back through the send, " + + "and a deleted send's tracked links stop redirecting; 90 days is 3x the widest window", + window: func(p RetentionPolicy) time.Duration { return p.Sends }, + batch: func(r Retainer) func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return r.PurgeSends + }, + }, + { + name: "task_dead_letters", setting: "INROAD_RETENTION_DEAD_LETTERS_DAYS", floor: 7 * day, scan: unguardedScan, + why: "a dead letter is only useful while someone can still triage and replay it; a week covers a long weekend plus an outage", + window: func(p RetentionPolicy) time.Duration { return p.DeadLetters }, + batch: func(r Retainer) func(context.Context, coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + return r.PurgeDeadLetters + }, + }, + } +} + +// RetentionDays is the operator's configuration in the unit it is stated in. +// Zero disables a table. +type RetentionDays struct { + DeliverabilityEvents, InboxThreads, TrackingEvents, Sends, DeadLetters int +} + +// RetentionPolicyFromDays converts whole days into the policy. A negative count +// becomes a negative window, which Validate refuses — it is not clamped to zero, +// because "disabled" must be something the operator wrote. +func RetentionPolicyFromDays(d RetentionDays) RetentionPolicy { + return RetentionPolicy{ + DeliverabilityEvents: time.Duration(d.DeliverabilityEvents) * day, + InboxThreads: time.Duration(d.InboxThreads) * day, + TrackingEvents: time.Duration(d.TrackingEvents) * day, + Sends: time.Duration(d.Sends) * day, + DeadLetters: time.Duration(d.DeadLetters) * day, + } +} + +// Validate refuses a window shorter than its table's floor, naming the setting +// and the reason — every violation at once, so one restart fixes them all (the +// same rule config.Load follows). A floor is not a recommendation: it is the +// point below which a rate the product acts on, or a live path, would start +// reading a table the sweep had already emptied. Disabled (zero) is always valid; +// a negative window is not. +func (p RetentionPolicy) Validate() error { + var errs []error + for _, t := range retentionTables() { + w := t.window(p) + switch { + case w == 0: + continue + case w < 0: + errs = append(errs, fmt.Errorf("%s must not be negative (0 disables %s retention)", t.setting, t.name)) + case w < t.floor: + errs = append(errs, fmt.Errorf("%s is %d days, below the %d-day minimum for %s: %s", + t.setting, days(w), days(t.floor), t.name, t.why)) + } + } + return errors.Join(errs...) +} + +// Enabled reports each table this policy sweeps, for the startup log line that +// answers "is retention on here, and for what" without reading the environment. +func (p RetentionPolicy) Enabled() map[string]int { + out := map[string]int{} + for _, t := range retentionTables() { + if w := t.window(p); w > 0 { + out[t.name] = days(w) + } + } + return out +} + +func days(d time.Duration) int { return int(d / day) } + +// RetentionOptions bounds one run. The zero value takes the defaults below. +type RetentionOptions struct { + // BatchSize, when set, overrides every table's scan bound (retentionTable.scan). + // Tests use it to cross batch boundaries with a handful of rows. + BatchSize int32 + // MaxBatchesPerTable caps one table's share of a run, so a backlog on one + // table cannot hold the job for the whole budget and starve the tables after + // it. A table left undrained is logged and resumes next run. + MaxBatchesPerTable int + // Budget is the wall time after which no NEW batch starts. It is a stop + // signal between batches, not a timeout on one: a batch is one statement and + // is never interrupted mid-flight except by the task's own context. + Budget time.Duration + // Now is the clock the budget is measured on; nil means time.Now. + Now func() time.Time +} + +const ( + // defaultMaxBatchesPerTable: 40 batches is 200k rows per unguarded table per + // run and 800k scanned per guarded one. The sweep runs hourly, so a table + // drains at up to 4.8M deletes a day — above the write rate of any + // single-node deployment's busiest table — while one run stays in the tens of + // seconds. + defaultMaxBatchesPerTable = 40 + // defaultRetentionBudget is half of queue.sweepTimeout (10 minutes), the + // ceiling asynq kills this handler at: a run that hits it stops cleanly and + // is recorded as ok, rather than being killed mid-batch and retried. + defaultRetentionBudget = 5 * time.Minute +) + +func (o RetentionOptions) withDefaults() RetentionOptions { + if o.MaxBatchesPerTable <= 0 { + o.MaxBatchesPerTable = defaultMaxBatchesPerTable + } + if o.Budget <= 0 { + o.Budget = defaultRetentionBudget + } + if o.Now == nil { + o.Now = time.Now + } + return o +} + +// RetentionHandler applies the policy to every enabled table, in order, in +// bounded batches, resuming each table from where the previous run stopped. +// +// Single-instance deployment-wide (TryRetentionSweepLock): a run that finds +// another in progress does nothing and succeeds, because the other one is doing +// the work. Every batch is still idempotent on its own — deletes by age on the +// database clock, rows taken with SKIP LOCKED — so a run killed mid-way leaves +// nothing half-done. +// +// A table that fails does not stop the tables after it — one table's error must +// not also freeze every other table's retention — but the run returns every +// failure joined, so asynq retries it and the ledger records the error. A retry +// is cheap: whatever the first attempt deleted is gone, and the rest resumes +// from the saved cursor. +func RetentionHandler(core Retainer, policy RetentionPolicy, m *metrics.Metrics, opts RetentionOptions) func(context.Context, *asynq.Task) error { + opts = opts.withDefaults() + return func(ctx context.Context, _ *asynq.Task) (runErr error) { + if len(policy.Enabled()) == 0 { + return nil + } + release, acquired, err := core.TryRetentionSweepLock(ctx) + if err != nil { + return err + } + if !acquired { + slog.InfoContext(ctx, "retention sweep already running elsewhere; this run does nothing") + return nil + } + defer func() { + if err := release(); err != nil { + runErr = errors.Join(runErr, err) + } + }() + + deadline := opts.Now().Add(opts.Budget) + var errs []error + for _, t := range retentionTables() { + window := t.window(policy) + if window <= 0 { + continue + } + if !opts.Now().Before(deadline) { + slog.WarnContext(ctx, "retention run budget spent before this table; it resumes next run", + "table", t.name, "budget", opts.Budget) + continue + } + if err := applyRetention(ctx, core, t, window, m, opts, deadline); err != nil { + if ctx.Err() != nil { + return errors.Join(append(errs, err)...) + } + errs = append(errs, err) + } + } + return errors.Join(errs...) + } +} + +// applyRetention runs one table's batches from its saved cursor until it drains, +// hits its batch cap, or the run's budget is spent — whichever comes first — +// saves where it stopped, and reports what it did. +// +// The cursor starts over from the oldest row when the table drains (so the next +// run re-checks rows a guard kept), and when a day has passed since it last +// started over (so a table too big to drain in one run still gets its kept rows +// re-checked daily rather than never). +func applyRetention(ctx context.Context, core Retainer, t retentionTable, window time.Duration, + m *metrics.Metrics, opts RetentionOptions, deadline time.Time) error { + scan := t.scan + if opts.BatchSize > 0 { + scan = opts.BatchSize + } + progress, err := core.LoadRetentionProgress(ctx, t.name) + if err != nil { + return fmt.Errorf("retention %s: %w", t.name, err) + } + cursor, newCycle := progress.Cursor, false + if progress.CycleExpired { + cursor, newCycle = coreapi.RetentionCursor{}, true + } + resumed := cursor != (coreapi.RetentionCursor{}) + + batch := t.batch(core) + var ( + total coreapi.RetentionBatch + batches int + drained bool + ) + for batches < opts.MaxBatchesPerTable && opts.Now().Before(deadline) { + var res coreapi.RetentionBatch + res, err = batch(ctx, coreapi.RetentionRequest{OlderThan: window, Limit: scan, After: cursor}) + if err != nil { + err = fmt.Errorf("retention %s: %w", t.name, err) + break + } + batches++ + total.Scanned += res.Scanned + total.Deleted += res.Deleted + total.Dependents += res.Dependents + total.RolledUp += res.RolledUp + if res.Scanned < int64(scan) { + drained = true + break + } + cursor = res.Next + } + // Saved even when the loop ended on an error: the cursor only ever advances + // past a batch that committed, so resuming from it loses nothing. + save := cursor + if drained { + save, newCycle = coreapi.RetentionCursor{}, true + } + if serr := core.SaveRetentionProgress(ctx, t.name, save, newCycle); serr != nil { + err = errors.Join(err, fmt.Errorf("retention %s: %w", t.name, serr)) + } + + // Recorded even when the loop ended on an error: rows earlier batches + // deleted are gone, and a counter that under-reported them would make the + // failing run look like it did nothing. + m.RetentionRowsDeleted(t.name, total.Deleted, total.Dependents) + attrs := []any{ + "table", t.name, "window_days", days(window), "scanned", total.Scanned, "rows", total.Deleted, + "dependent_rows", total.Dependents, "batches", batches, "drained", drained, "resumed", resumed, + } + if total.RolledUp > 0 { + attrs = append(attrs, "rollup_rows", total.RolledUp) + } + switch { + case err != nil: + slog.ErrorContext(ctx, "retention batch failed", append(attrs, "err", err)...) + case !drained: + // Not an error: the backlog resumes next run from the saved cursor. But a + // table that is never drained is a window the sweep is not keeping, so it + // must be visible at the default log level. + slog.WarnContext(ctx, "retention backlog not drained this run; it resumes next run", attrs...) + default: + slog.InfoContext(ctx, "retention applied", attrs...) + } + return err +} diff --git a/internal/worker/maintenance/retention_test.go b/internal/worker/maintenance/retention_test.go new file mode 100644 index 00000000..63a2d088 --- /dev/null +++ b/internal/worker/maintenance/retention_test.go @@ -0,0 +1,543 @@ +package maintenance + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "log/slog" + "strings" + "testing" + "time" + + "github.com/google/uuid" + "github.com/hibiken/asynq" + + "github.com/inroad/inroad/internal/coreapi" + "github.com/inroad/inroad/internal/platform/metrics" + "github.com/inroad/inroad/internal/platform/metrics/metricstest" + "github.com/inroad/inroad/internal/platform/queue" +) + +// fakeTable scripts one table's batches: backlog rows wait to be deleted, at most +// Limit per call, and every call's request is recorded so a test can assert what +// the handler asked for (the window, the limit, the cursor it resumed from). +type fakeTable struct { + backlog int64 + kept int64 // rows per batch a guard keeps: scanned, not deleted + dependents int64 // per deleted row + err error + errAfter int // fail on this call number (1-based); 0 = never + calls []coreapi.RetentionRequest + onCall func() +} + +func (f *fakeTable) batch(_ context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + f.calls = append(f.calls, req) + if f.onCall != nil { + f.onCall() + } + if f.err != nil && (f.errAfter == 0 || len(f.calls) == f.errAfter) { + return coreapi.RetentionBatch{}, f.err + } + n := min(f.backlog, int64(req.Limit)) + f.backlog -= n + deleted := max(n-f.kept, 0) + // The cursor encodes the call number, so a test can check the NEXT request + // resumed from exactly the cursor THIS batch returned. + next := coreapi.RetentionCursor{At: time.Unix(int64(len(f.calls)), 0).UTC(), ID: uuid.New()} + return coreapi.RetentionBatch{Scanned: n, Deleted: deleted, Dependents: deleted * f.dependents, Next: next}, nil +} + +type savedProgress struct { + cursor coreapi.RetentionCursor + newCycle bool +} + +type fakeRetainer struct { + deliverability, inbox, tracking, sends, deadLetters fakeTable + order []string + + progress map[string]coreapi.RetentionProgress // what Load returns + saved map[string]savedProgress // what Save recorded + + lockBusy bool // another run holds the lock + lockErr error + lockTries int + releases int + releaseErr error +} + +func (f *fakeRetainer) LoadRetentionProgress(_ context.Context, table string) (coreapi.RetentionProgress, error) { + return f.progress[table], nil +} + +func (f *fakeRetainer) SaveRetentionProgress(_ context.Context, table string, cursor coreapi.RetentionCursor, newCycle bool) error { + if f.saved == nil { + f.saved = map[string]savedProgress{} + } + f.saved[table] = savedProgress{cursor: cursor, newCycle: newCycle} + return nil +} + +func (f *fakeRetainer) TryRetentionSweepLock(context.Context) (func() error, bool, error) { + f.lockTries++ + if f.lockErr != nil { + return nil, false, f.lockErr + } + if f.lockBusy { + return nil, false, nil + } + return func() error { f.releases++; return f.releaseErr }, true, nil +} + +func (f *fakeRetainer) PurgeDeliverabilityEvents(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + f.order = append(f.order, "deliverability_events") + return f.deliverability.batch(ctx, req) +} + +func (f *fakeRetainer) PurgeInboxThreads(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + f.order = append(f.order, "inbox_threads") + return f.inbox.batch(ctx, req) +} + +func (f *fakeRetainer) RollupTrackingEvents(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + f.order = append(f.order, "tracking_events") + return f.tracking.batch(ctx, req) +} + +func (f *fakeRetainer) PurgeSends(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + f.order = append(f.order, "sends") + return f.sends.batch(ctx, req) +} + +func (f *fakeRetainer) PurgeDeadLetters(ctx context.Context, req coreapi.RetentionRequest) (coreapi.RetentionBatch, error) { + f.order = append(f.order, "task_dead_letters") + return f.deadLetters.batch(ctx, req) +} + +var _ Retainer = (*fakeRetainer)(nil) + +func allEnabled() RetentionPolicy { + return RetentionPolicy{ + DeliverabilityEvents: 400 * day, InboxThreads: 401 * day, TrackingEvents: 402 * day, + Sends: 403 * day, DeadLetters: 90 * day, + } +} + +func runRetention(t *testing.T, core Retainer, p RetentionPolicy, m *metrics.Metrics, opts RetentionOptions) error { + t.Helper() + return RetentionHandler(core, p, m, opts)(context.Background(), asynq.NewTask(queue.TaskMaintenanceRetention, nil)) +} + +// The default: every recipient-identifying table disabled. A disabled table must +// cost NOTHING — not one call. A handler that "checked" a disabled table with a +// zero window would be one refactor away from `now() - 0s`, i.e. every row. +func TestRetentionDisabledPolicyCallsNothing(t *testing.T) { + core := &fakeRetainer{} + if err := runRetention(t, core, RetentionPolicy{}, nil, RetentionOptions{}); err != nil { + t.Fatalf("handler: %v", err) + } + if len(core.order) != 0 { + t.Fatalf("a fully disabled policy made %d calls (%v), want none", len(core.order), core.order) + } + if core.lockTries != 0 { + t.Fatal("a fully disabled policy took the sweep lock; it should touch the database not at all") + } +} + +// Another replica's run holds the lock: this one must do nothing and SUCCEED — +// the other run is doing the work, and an error would make asynq retry a run +// that has nothing to do. +func TestRetentionDoesNothingWhileAnotherRunHoldsTheLock(t *testing.T) { + core := &fakeRetainer{lockBusy: true, sends: fakeTable{backlog: 10}} + if err := runRetention(t, core, allEnabled(), nil, RetentionOptions{}); err != nil { + t.Fatalf("handler: %v", err) + } + if len(core.order) != 0 { + t.Fatalf("a run without the lock swept %v", core.order) + } +} + +// The lock is released exactly once, including when a table failed — a lock +// left held would make every later run on every replica skip. +func TestRetentionReleasesTheLockEvenWhenATableFails(t *testing.T) { + boom := errors.New("statement timeout") + core := &fakeRetainer{sends: fakeTable{err: boom}} + err := runRetention(t, core, allEnabled(), nil, RetentionOptions{}) + if !errors.Is(err, boom) { + t.Fatalf("handler error = %v, want %v", err, boom) + } + if core.releases != 1 { + t.Fatalf("lock released %d times, want exactly 1", core.releases) + } +} + +// A failure to take or release the lock is a failure of the run. +func TestRetentionLockErrorsFailTheRun(t *testing.T) { + lockErr := errors.New("connection refused") + if err := runRetention(t, &fakeRetainer{lockErr: lockErr}, allEnabled(), nil, RetentionOptions{}); !errors.Is(err, lockErr) { + t.Fatalf("lock error: handler = %v, want %v", err, lockErr) + } + releaseErr := errors.New("unlock failed") + if err := runRetention(t, &fakeRetainer{releaseErr: releaseErr}, allEnabled(), nil, RetentionOptions{}); !errors.Is(err, releaseErr) { + t.Fatalf("release error: handler = %v, want %v", err, releaseErr) + } +} + +// Across runs: a table that did not drain saves where it stopped, and the next +// run resumes there instead of re-reading every row before it. +func TestRetentionResumesTheNextRunFromTheSavedCursor(t *testing.T) { + core := &fakeRetainer{sends: fakeTable{backlog: 100}} + opts := RetentionOptions{BatchSize: 10, MaxBatchesPerTable: 2} + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, opts); err != nil { + t.Fatalf("first run: %v", err) + } + saved := core.saved["sends"] + if saved.cursor.At != time.Unix(2, 0).UTC() || saved.newCycle { + t.Fatalf("saved after an undrained run = %+v, want the second batch's cursor and no new cycle", saved) + } + + core.progress = map[string]coreapi.RetentionProgress{"sends": {Cursor: saved.cursor}} + core.sends.calls = nil + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, opts); err != nil { + t.Fatalf("second run: %v", err) + } + if got := core.sends.calls[0].After; got != saved.cursor { + t.Fatalf("second run started from %+v, want the saved %+v", got, saved.cursor) + } +} + +// A drained table starts over from the oldest row next run, with a new cycle — +// rows a guard kept behind the cursor are re-checked. +func TestRetentionStartsOverAfterDraining(t *testing.T) { + core := &fakeRetainer{ + sends: fakeTable{backlog: 3}, + progress: map[string]coreapi.RetentionProgress{"sends": {Cursor: coreapi.RetentionCursor{At: time.Unix(99, 0).UTC()}}}, + } + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, RetentionOptions{BatchSize: 10}); err != nil { + t.Fatalf("handler: %v", err) + } + if got := core.saved["sends"]; got.cursor != (coreapi.RetentionCursor{}) || !got.newCycle { + t.Fatalf("saved after draining = %+v, want the zero cursor and a new cycle", got) + } +} + +// A cycle older than a day starts over from the oldest row even though the +// table never drained — otherwise a table too large to drain in a run would +// never revisit the rows behind its cursor. +func TestRetentionStartsOverWhenTheCycleExpires(t *testing.T) { + core := &fakeRetainer{ + sends: fakeTable{backlog: 1000}, + progress: map[string]coreapi.RetentionProgress{"sends": { + Cursor: coreapi.RetentionCursor{At: time.Unix(99, 0).UTC()}, CycleExpired: true, + }}, + } + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, RetentionOptions{BatchSize: 10, MaxBatchesPerTable: 1}); err != nil { + t.Fatalf("handler: %v", err) + } + if got := core.sends.calls[0].After; got != (coreapi.RetentionCursor{}) { + t.Fatalf("an expired cycle resumed from %+v, want the oldest row", got) + } + if !core.saved["sends"].newCycle { + t.Fatal("an expired cycle was not restarted") + } +} + +// A batch whose every scanned row was KEPT is not the end of the table: the +// drain signal is rows scanned, not rows deleted. Judging by deletes would stop +// the sweep at the first run of kept rows and never reach the rows behind it. +func TestRetentionAnAllKeptBatchIsNotTakenAsDrained(t *testing.T) { + core := &fakeRetainer{sends: fakeTable{backlog: 25, kept: 10}} + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, RetentionOptions{BatchSize: 10}); err != nil { + t.Fatalf("handler: %v", err) + } + if got := len(core.sends.calls); got != 3 { + t.Fatalf("calls = %d, want 3 (10 kept, 10 kept, 5 short) — kept rows must not end the walk", got) + } +} + +// Only the enabled table is swept, with exactly its own window — windows are +// per-table, so a handler that passed one table's window to another would be a +// silent policy violation. +func TestRetentionSweepsOnlyEnabledTablesWithTheirOwnWindow(t *testing.T) { + core := &fakeRetainer{} + p := RetentionPolicy{Sends: 500 * day, DeadLetters: 30 * day} + if err := runRetention(t, core, p, nil, RetentionOptions{}); err != nil { + t.Fatalf("handler: %v", err) + } + for name, tc := range map[string]struct { + calls []coreapi.RetentionRequest + want time.Duration + }{ + "deliverability": {core.deliverability.calls, 0}, + "inbox": {core.inbox.calls, 0}, + "tracking": {core.tracking.calls, 0}, + "sends": {core.sends.calls, 500 * day}, + "dead letters": {core.deadLetters.calls, 30 * day}, + } { + if tc.want == 0 { + if len(tc.calls) != 0 { + t.Errorf("%s is disabled but was called %d times", name, len(tc.calls)) + } + continue + } + if len(tc.calls) != 1 { + t.Fatalf("%s: %d calls, want 1 (an empty backlog drains on the first batch)", name, len(tc.calls)) + } + if tc.calls[0].OlderThan != tc.want { + t.Errorf("%s window = %s, want %s", name, tc.calls[0].OlderThan, tc.want) + } + } +} + +// Order is load-bearing (see retentionTables): the tables that hold a send back +// run before sends, and tracking rolls up before a send delete would cascade it. +func TestRetentionRunsTablesInDependencyOrder(t *testing.T) { + core := &fakeRetainer{} + if err := runRetention(t, core, allEnabled(), nil, RetentionOptions{}); err != nil { + t.Fatalf("handler: %v", err) + } + want := []string{"deliverability_events", "inbox_threads", "tracking_events", "sends", "task_dead_letters"} + if strings.Join(core.order, ",") != strings.Join(want, ",") { + t.Fatalf("order = %v, want %v", core.order, want) + } +} + +// Batch boundaries. 12 rows at a batch of 5 is 5, 5, 2: the third batch is short, +// which is the drained signal, so there is no fourth call. Each batch after the +// first must resume from the cursor the one before it returned — resuming from +// the start would re-walk every row a guard kept, every batch. +func TestRetentionBatchesUntilAShortBatchAndResumesFromTheCursor(t *testing.T) { + core := &fakeRetainer{sends: fakeTable{backlog: 12}} + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, RetentionOptions{BatchSize: 5}); err != nil { + t.Fatalf("handler: %v", err) + } + calls := core.sends.calls + if len(calls) != 3 { + t.Fatalf("calls = %d, want 3 (5 + 5 + a short 2)", len(calls)) + } + if !calls[0].After.At.IsZero() || calls[0].After.ID != uuid.Nil { + t.Errorf("first batch cursor = %+v, want the zero cursor (before every row)", calls[0].After) + } + for i := 1; i < len(calls); i++ { + if got, want := calls[i].After.At, time.Unix(int64(i), 0).UTC(); !got.Equal(want) { + t.Errorf("batch %d resumed from %v, want the cursor batch %d returned (%v)", i+1, got, i, want) + } + if calls[i].Limit != 5 { + t.Errorf("batch %d limit = %d, want 5", i+1, calls[i].Limit) + } + } + if core.sends.backlog != 0 { + t.Errorf("backlog left = %d, want 0", core.sends.backlog) + } +} + +// An exact multiple of the batch size needs one more, empty, batch to learn it +// is drained — a full batch never means "done". +func TestRetentionAFullLastBatchIsNotTakenAsDrained(t *testing.T) { + core := &fakeRetainer{sends: fakeTable{backlog: 10}} + if err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, nil, RetentionOptions{BatchSize: 5}); err != nil { + t.Fatalf("handler: %v", err) + } + if got := len(core.sends.calls); got != 3 { + t.Fatalf("calls = %d, want 3 (5, 5, then an empty batch that proves it drained)", got) + } +} + +// One table's backlog must not take the whole run: it stops at its batch cap, +// the tables after it still run, and the undrained table is reported at WARN. +func TestRetentionCapsOneTableSoTheRestStillRun(t *testing.T) { + core := &fakeRetainer{deliverability: fakeTable{backlog: 1_000}, deadLetters: fakeTable{backlog: 1}} + logs := captureLogs(t) + err := runRetention(t, core, allEnabled(), nil, RetentionOptions{BatchSize: 10, MaxBatchesPerTable: 3}) + if err != nil { + t.Fatalf("handler: %v", err) + } + if got := len(core.deliverability.calls); got != 3 { + t.Errorf("deliverability calls = %d, want the cap of 3", got) + } + if len(core.deadLetters.calls) != 1 || core.deadLetters.backlog != 0 { + t.Errorf("dead letters after a capped table: calls=%d backlog=%d, want it still swept", len(core.deadLetters.calls), core.deadLetters.backlog) + } + rec := findLog(t, logs, "retention backlog not drained this run; it resumes next run", "deliverability_events") + if rec == nil { + t.Fatal("an undrained table was not reported") + } + if rec["level"] != "WARN" || rec["rows"] != float64(30) || rec["drained"] != false { + t.Errorf("undrained record = %v, want WARN rows=30 drained=false", rec) + } +} + +// The budget is checked between batches, on the injected clock. Once it is spent +// no new batch starts — in this table or any later one — and the run still ends +// cleanly: a spent budget is "resume next hour", not a failure to retry. +func TestRetentionStopsStartingBatchesOnceTheBudgetIsSpent(t *testing.T) { + now := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC) + clock := func() time.Time { return now } + core := &fakeRetainer{inbox: fakeTable{backlog: 1_000}} + // Every inbox batch advances the clock by a minute; the budget is 3 minutes. + core.inbox.onCall = func() { now = now.Add(time.Minute) } + + err := runRetention(t, core, allEnabled(), nil, RetentionOptions{BatchSize: 10, Budget: 3 * time.Minute, Now: clock}) + if err != nil { + t.Fatalf("a spent budget must not fail the run: %v", err) + } + if got := len(core.inbox.calls); got != 3 { + t.Errorf("inbox calls = %d, want 3 (one per minute of a 3-minute budget)", got) + } + if len(core.tracking.calls)+len(core.sends.calls)+len(core.deadLetters.calls) != 0 { + t.Error("a table after the budget was spent still started a batch") + } + if len(core.deliverability.calls) != 1 { + t.Error("the table before the budget was spent should have run") + } +} + +// A failing table must not freeze every other table's retention, but the run +// must still fail so asynq retries it and the job ledger records why. +func TestRetentionOneFailingTableDoesNotStopTheOthers(t *testing.T) { + boom := errors.New("statement timeout") + core := &fakeRetainer{tracking: fakeTable{err: boom}, sends: fakeTable{backlog: 3}} + err := runRetention(t, core, allEnabled(), nil, RetentionOptions{}) + if !errors.Is(err, boom) { + t.Fatalf("handler error = %v, want it to carry the tracking failure", err) + } + if !strings.Contains(err.Error(), "tracking_events") { + t.Errorf("error %q does not name the failing table", err) + } + if core.sends.backlog != 0 || len(core.deadLetters.calls) != 1 { + t.Error("the tables after the failing one were not swept") + } +} + +// Rows an earlier batch deleted are gone even when a later batch fails, so the +// metric must count them — otherwise a failing run reads as one that did nothing. +func TestRetentionCountsRowsDeletedBeforeAFailure(t *testing.T) { + boom := errors.New("connection reset") + core := &fakeRetainer{sends: fakeTable{backlog: 100, dependents: 2, err: boom, errAfter: 3}} + m := metrics.New() + err := runRetention(t, core, RetentionPolicy{Sends: 100 * day}, m, RetentionOptions{BatchSize: 10}) + if !errors.Is(err, boom) { + t.Fatalf("handler error = %v, want %v", err, boom) + } + families := metricstest.Scrape(t, m) + if got := metricstest.CounterValue(families, "inroad_retention_rows_deleted_total", + map[string]string{"table": "sends", "scope": "table"}); got != 20 { + t.Errorf("sends rows counted = %v, want 20 (two successful batches before the failure)", got) + } + if got := metricstest.CounterValue(families, "inroad_retention_rows_deleted_total", + map[string]string{"table": "sends", "scope": "dependent"}); got != 40 { + t.Errorf("sends dependents counted = %v, want 40", got) + } +} + +// A cancelled context ends the run instead of trying every remaining table +// against a dead connection. +func TestRetentionStopsOnACancelledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + core := &fakeRetainer{} + core.deliverability.onCall = cancel + core.deliverability.err = context.Canceled + err := RetentionHandler(core, allEnabled(), nil, RetentionOptions{})(ctx, asynq.NewTask(queue.TaskMaintenanceRetention, nil)) + if !errors.Is(err, context.Canceled) { + t.Fatalf("handler error = %v, want context.Canceled", err) + } + if len(core.order) != 1 { + t.Fatalf("calls after cancellation: %v, want only the first", core.order) + } +} + +// The log line is the per-run record an operator reads; it must carry the real +// numbers per table. +func TestRetentionLogsWhatEachTableDid(t *testing.T) { + core := &fakeRetainer{inbox: fakeTable{backlog: 4, dependents: 3}} + logs := captureLogs(t) + if err := runRetention(t, core, RetentionPolicy{InboxThreads: 45 * day}, nil, RetentionOptions{}); err != nil { + t.Fatalf("handler: %v", err) + } + rec := findLog(t, logs, "retention applied", "inbox_threads") + if rec == nil { + t.Fatal("no retention record for inbox_threads") + } + if rec["rows"] != float64(4) || rec["dependent_rows"] != float64(12) || rec["window_days"] != float64(45) || rec["drained"] != true { + t.Errorf("record = %v, want rows=4 dependent_rows=12 window_days=45 drained=true", rec) + } +} + +func TestRetentionPolicyValidate(t *testing.T) { + for _, tc := range []struct { + name string + policy RetentionPolicy + wantErr []string // substrings; empty means valid + }{ + {name: "everything disabled is valid", policy: RetentionPolicy{}}, + {name: "every floor exactly is valid", policy: RetentionPolicy{ + DeliverabilityEvents: 90 * day, InboxThreads: 30 * day, TrackingEvents: 30 * day, Sends: 90 * day, DeadLetters: 7 * day, + }}, + {name: "one day under each floor", policy: RetentionPolicy{ + DeliverabilityEvents: 89 * day, InboxThreads: 29 * day, TrackingEvents: 29 * day, Sends: 89 * day, DeadLetters: 6 * day, + }, wantErr: []string{ + "INROAD_RETENTION_DELIVERABILITY_EVENTS_DAYS is 89 days, below the 90-day minimum", + "INROAD_RETENTION_INBOX_DAYS is 29 days, below the 30-day minimum", + "INROAD_RETENTION_TRACKING_EVENTS_DAYS is 29 days, below the 30-day minimum", + "INROAD_RETENTION_SENDS_DAYS is 89 days, below the 90-day minimum", + "INROAD_RETENTION_DEAD_LETTERS_DAYS is 6 days, below the 7-day minimum", + }}, + {name: "negative is refused, not read as disabled", policy: RetentionPolicy{Sends: -day}, + wantErr: []string{"INROAD_RETENTION_SENDS_DAYS must not be negative"}}, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.policy.Validate() + if len(tc.wantErr) == 0 { + if err != nil { + t.Fatalf("Validate() = %v, want nil", err) + } + return + } + if err == nil { + t.Fatal("Validate() = nil, want an error") + } + for _, want := range tc.wantErr { + if !strings.Contains(err.Error(), want) { + t.Errorf("error does not report %q:\n%v", want, err) + } + } + }) + } +} + +func TestRetentionPolicyFromDaysMapsEachTable(t *testing.T) { + p := RetentionPolicyFromDays(RetentionDays{DeliverabilityEvents: 1, InboxThreads: 2, TrackingEvents: 3, Sends: 4, DeadLetters: 5}) + want := RetentionPolicy{DeliverabilityEvents: day, InboxThreads: 2 * day, TrackingEvents: 3 * day, Sends: 4 * day, DeadLetters: 5 * day} + if p != want { + t.Fatalf("policy = %+v, want %+v", p, want) + } + if got := (RetentionPolicy{Sends: 400 * day}).Enabled(); len(got) != 1 || got["sends"] != 400 { + t.Errorf("Enabled() = %v, want map[sends:400]", got) + } +} + +func captureLogs(t *testing.T) *bytes.Buffer { + t.Helper() + restore := slog.Default() + var logs bytes.Buffer + slog.SetDefault(slog.New(slog.NewJSONHandler(&logs, &slog.HandlerOptions{Level: slog.LevelDebug}))) + t.Cleanup(func() { slog.SetDefault(restore) }) + return &logs +} + +// findLog returns the record with this message for this table, or nil. +func findLog(t *testing.T, logs *bytes.Buffer, msg, table string) map[string]any { + t.Helper() + for line := range bytes.SplitSeq(bytes.TrimSpace(logs.Bytes()), []byte("\n")) { + var rec map[string]any + if err := json.Unmarshal(line, &rec); err != nil { + t.Fatalf("log line is not JSON (%s): %v", line, err) + } + if rec["msg"] == msg && rec["table"] == table { + return rec + } + } + return nil +} diff --git a/internal/worker/role.go b/internal/worker/role.go index dbcc7667..2155ea1c 100644 --- a/internal/worker/role.go +++ b/internal/worker/role.go @@ -18,7 +18,7 @@ const ( // self-host topology: an operator running Inroad for their own mailboxes has // one trust domain and should never have to learn this setting exists. RoleAll Role = "all" - // RoleControl runs the scheduler, the eight periodic sweeps and the purges. + // RoleControl runs the scheduler, the nine periodic sweeps and the purges. // It stays on trusted infrastructure beside the API. RoleControl Role = "control" // RoleSend runs per-message work only: sends, warmup ticks, inbox polls,