From 0b7618d8588b9d2d23e45838ecb6ea0afd172e01 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Tue, 28 Jul 2026 22:00:28 +0200 Subject: [PATCH] clarify naming conventions --- docs/plan/consistency-check.md | 62 +++++++++++++++++----------------- docs/plan/job-registry.md | 48 +++++++++++++------------- 2 files changed, 55 insertions(+), 55 deletions(-) diff --git a/docs/plan/consistency-check.md b/docs/plan/consistency-check.md index b2b17fdc..3ea9d635 100644 --- a/docs/plan/consistency-check.md +++ b/docs/plan/consistency-check.md @@ -38,14 +38,14 @@ This plan lands: compiles without declaring its consistency contract. 3. An **educational surface** in trait doc-comments — decision axes (severity, direction, grace, cursor) and canonical-example pointers. -4. **Consistency-specific persistence** — `admin.consistency_findings`, +4. **Consistency-specific persistence** — `jobs.run_findings`, idempotent-on-`(run_id, kind, resource_id)`. 5. A **first check** — `BlobConsistencyCheck` (both directions, blob-keyed cursor, severity split). **Layer boundary — the runtime is not in this plan.** The resumable execution engine (cursor persistence, exclusivity, cancel protocol, -crash recovery, `admin.background_runs` schema, `JobStore`, +crash recovery, `jobs.recoverable_runs` schema, `JobStore`, `RunOutcome`, `run_or_resume`) lives in `docs/plan/job-registry.md` Part 2. This plan describes what `ConsistencyCheck` implementors write and how the check-specific bits (findings, severity, @@ -53,7 +53,7 @@ write and how the check-specific bits (findings, severity, **Order:** ships **after** the job-registry Part 2 engine lands. Consistency closes an operator-visibility gap today, but it depends -on Part 2's `RecoverableJob` + `JobStore` + `admin.background_runs` +on Part 2's `RecoverableJobHandler` + `JobStore` + `jobs.recoverable_runs` primitives — those come first. Once both are in, consistency runs are admin-triggered v1, becoming periodic-triggered when a `JobRegistry` (Part 1) tenant wraps `run_or_resume` for each @@ -157,11 +157,11 @@ Race matrix — missing direction: Nothing before "byte-exact whole-table snapshot verification" needs a quiescent server. Reserve `concurrent_safe() = false` for that one. -### Resumability — runs live in Part 2's `background_runs` +### Resumability — runs live in Part 2's `recoverable_runs` -Consistency runs are ordinary `RecoverableJob`s. The runtime plumbing +Consistency runs are ordinary `RecoverableJobHandler`s. The runtime plumbing — cursor persistence, exclusivity, cancel protocol, crash recovery, -`admin.background_runs` schema, `JobStore` trait, `RunOutcome`, +`jobs.recoverable_runs` schema, `JobStore` trait, `RunOutcome`, `run_or_resume` helper — lives in `docs/plan/job-registry.md` Part 2. This plan does not redefine any of it. @@ -172,10 +172,10 @@ This plan does not redefine any of it. `SELECT DISTINCT ON (job_name)` return the last run of every check alongside every other background job. - Consistency's per-check knobs — `grace_window_secs`, `batch_size`, - `concurrent_safe` — live inside `background_runs.params` JSONB at + `concurrent_safe` — live inside `recoverable_runs.params` JSONB at run-start time. The check reads them back via `serde_json::from_value(store.params()?)`. -- `background_runs.stats` accumulates `{"scanned_count": …, +- `recoverable_runs.stats` accumulates `{"scanned_count": …, "findings_this_run": …}`; readers call `(stats->>'scanned_count')::bigint`. @@ -183,9 +183,9 @@ The findings themselves are Layer C (this plan) — they don't generalise to storage-migration or reextract: ```sql -CREATE TABLE admin.consistency_findings ( +CREATE TABLE jobs.run_findings ( id UUID PRIMARY KEY, - run_id UUID NOT NULL REFERENCES admin.background_runs(id) ON DELETE CASCADE, + run_id UUID NOT NULL REFERENCES jobs.recoverable_runs(id) ON DELETE CASCADE, kind TEXT NOT NULL, -- OrphanBlob / MissingBlob / ... severity TEXT NOT NULL, -- DataLoss / Reclaimable / ... resource_id TEXT NOT NULL, @@ -193,15 +193,15 @@ CREATE TABLE admin.consistency_findings ( found_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), UNIQUE (run_id, kind, resource_id) -- idempotent re-scan on resume ); -CREATE INDEX ON admin.consistency_findings (run_id, severity); +CREATE INDEX ON jobs.run_findings (run_id, severity); ``` -FK on `background_runs.id` links a finding back to the run that +FK on `recoverable_runs.id` links a finding back to the run that produced it; `ON DELETE CASCADE` clears findings when their run row is pruned by a future retention job. -`admin.*` is a NEW schema, created by Part 2's migration — keep it -distinct from `auth.*` / `storage.*` so operational tables don't +`jobs.*` is a NEW schema, created by Part 2's migration — keep it +distinct from `auth.*` / `storage.*` / `admin.*` so operational tables don't pollute domain schemas. ### Non-obvious traps @@ -223,7 +223,7 @@ learned the hard way in similar systems: transitioned (was `MissingBlob`, blob has since landed → drop the finding, not the whole run). 4. **Cooperative cancellation ONLY.** Between batches, poll - `background_runs.status`. A `tokio::spawn` abort mid-batch leaks — + `recoverable_runs.status`. A `tokio::spawn` abort mid-batch leaks — cursor unpersisted, findings half-written. Cancel path writes `status='Paused'` + current cursor before returning. 5. **Crash recovery on boot.** Any `status='Running'` at server start = @@ -484,7 +484,7 @@ impl ConsistencyRegistry { ## Admin surface -Consistency runs are ordinary `RecoverableJob`s (see +Consistency runs are ordinary `RecoverableJobHandler`s (see `docs/plan/job-registry.md` Part 2), so most operator actions reach them through the shared scheduler surface: @@ -510,7 +510,7 @@ GET /api/admin/jobs/consistency_{name}/runs/{id} ``` Findings enrichment on `runs/{id}` is consistency-specific — read -from `admin.consistency_findings` and joined into the response. +from `jobs.run_findings` and joined into the response. Everything else is generic Part 2 behaviour. Production surface — always on, audit-logged. No feature-flag gate. @@ -528,22 +528,22 @@ Production surface — always on, audit-logged. No feature-flag gate. `src/infrastructure/services/consistency/mod.rs` - `ConsistencyRegistry` (data structure only). - `PgCheckStore` — impl of `CheckStore` reading/writing - `admin.background_runs` (filtered to `job_name LIKE 'consistency_%'`) - + `admin.consistency_findings`. + `jobs.recoverable_runs` (filtered to `job_name LIKE 'consistency_%'`) + + `jobs.run_findings`. - `run_check(check, cursor, store)` — the runner that calls `run_resumable`, applies timeout, records outcome. ### 2. Schema migration `migrations/YYYYMMDDHHMMSS_background_runs_admin_schema.sql` — creates -the merged `admin.background_runs` table shared with the JobRegistry -plan. Consistency checks own the `admin.consistency_findings` table -alone and reference `background_runs.id` via FK. +the merged `jobs.recoverable_runs` table shared with the JobRegistry +plan. Consistency checks own the `jobs.run_findings` table +alone and reference `recoverable_runs.id` via FK. ```sql CREATE SCHEMA IF NOT EXISTS admin; -CREATE TABLE admin.background_runs ( +CREATE TABLE jobs.recoverable_runs ( id UUID PRIMARY KEY, job_name TEXT NOT NULL, -- 'consistency_blobs', 'storage_migration', 'reextract_audio', ... status TEXT NOT NULL, -- Running / Paused / Completed / Failed / CancelRequested @@ -556,13 +556,13 @@ CREATE TABLE admin.background_runs ( error_message TEXT ); CREATE UNIQUE INDEX one_active_run_per_job - ON admin.background_runs (job_name) + ON jobs.recoverable_runs (job_name) WHERE status IN ('Running', 'Paused'); -CREATE INDEX ON admin.background_runs (last_progress_at) WHERE status = 'Running'; +CREATE INDEX ON jobs.recoverable_runs (last_progress_at) WHERE status = 'Running'; -CREATE TABLE admin.consistency_findings ( +CREATE TABLE jobs.run_findings ( id UUID PRIMARY KEY, - run_id UUID NOT NULL REFERENCES admin.background_runs(id) ON DELETE CASCADE, + run_id UUID NOT NULL REFERENCES jobs.recoverable_runs(id) ON DELETE CASCADE, kind TEXT NOT NULL, severity TEXT NOT NULL, resource_id TEXT NOT NULL, @@ -570,7 +570,7 @@ CREATE TABLE admin.consistency_findings ( found_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), UNIQUE (run_id, kind, resource_id) ); -CREATE INDEX ON admin.consistency_findings (run_id, severity); +CREATE INDEX ON jobs.run_findings (run_id, severity); ``` ### 3. Supertrait bounds on existing state-owning ports @@ -631,7 +631,7 @@ Ship this PR without the actual checks. Grep `TODO(consistency)` = punch list. `src/interfaces/api/handlers/admin_handler.rs` - `start_consistency_check(name, force)` — insert an - `admin.background_runs` row with `job_name = 'consistency_'` + `jobs.recoverable_runs` row with `job_name = 'consistency_'` and `status = 'Running'`, spawn a tokio task calling `run_check`, return `run_id`. Concurrent triggers hit the partial unique index and short-circuit to returning the surviving row. @@ -651,7 +651,7 @@ In `AppServiceFactory` init, after DB pool is up: ```rust sqlx::query!( - "UPDATE admin.background_runs + "UPDATE jobs.recoverable_runs SET status = 'Paused', error_message = COALESCE(error_message, 'server restart mid-run') WHERE job_name LIKE 'consistency_%' @@ -660,7 +660,7 @@ sqlx::query!( ``` Filtering on `job_name LIKE 'consistency_%'` scopes the sweep to -consistency runs; other tenants of `background_runs` (storage +consistency runs; other tenants of `recoverable_runs` (storage migration, reextract-*) run the same auto-Pause sweep from their own boot-time hook. The JobRegistry supervisor's boot check may generalise this into a single scheduler-wide sweep — until then, one diff --git a/docs/plan/job-registry.md b/docs/plan/job-registry.md index 19adc5b3..cac45bb9 100644 --- a/docs/plan/job-registry.md +++ b/docs/plan/job-registry.md @@ -115,8 +115,8 @@ are already there. Keeping it as its own loop is right. of trash-cleanup + storage-usage + db_pool_monitor + dedup GC + grant-cleanup + tree-etag flush + content-index. High mechanical payoff, zero new schema, minimal review surface. -2. **Part 2 lands next** — introduces `admin.background_runs` schema - + `RecoverableJob` trait + `JobStore` port + `run_or_resume`. On +2. **Part 2 lands next** — introduces `jobs.recoverable_runs` schema + + `RecoverableJobHandler` trait + `JobStore` port + `run_or_resume`. On its own PR (schema change deserves independent review). 3. **Consistency-check framework (`docs/plan/consistency-check.md`)** lands third, consuming Part 2 as its runtime. @@ -432,16 +432,16 @@ into the scheduler. ## Part 2 — Recoverable-Run Engine -### Contract — `RecoverableJob` trait +### Contract — `RecoverableJobHandler` trait Sibling to `JobHandler`, NOT a subtrait. A stateless job that only implements `JobHandler` never needs to know Part 2 exists. ```rust #[async_trait] -pub trait RecoverableJob: Send + Sync { +pub trait RecoverableJobHandler: Send + Sync { /// Stable snake_case identifier — matches the `job_name` column - /// in `admin.background_runs`. + /// in `jobs.recoverable_runs`. fn name(&self) -> &str; /// Long-running, cooperative scan. The store is the job's ONLY @@ -479,13 +479,13 @@ pub enum RunOutcome { ### `JobStore` trait The port the engine passes to a recoverable job. Backed by -`admin.background_runs` in production; can be mocked for unit tests. +`jobs.recoverable_runs` in production; can be mocked for unit tests. ```rust #[async_trait] pub trait JobStore: Send + Sync { /// The `run_id` this handler was invoked with. Uniquely identifies - /// the row in `admin.background_runs`. + /// the row in `jobs.recoverable_runs`. fn run_id(&self) -> Uuid; /// Fixed at run start; used by consistency checks (and any other @@ -517,12 +517,12 @@ instance) are separate traits the impl composes on top of `JobStore`. `JobStore` itself carries no findings/severity concept — those are Layer C in the consistency-check plan, not the engine's concern. -### Schema — `admin.background_runs` +### Schema — `jobs.recoverable_runs` ```sql CREATE SCHEMA IF NOT EXISTS admin; -CREATE TABLE admin.background_runs ( +CREATE TABLE jobs.recoverable_runs ( id UUID PRIMARY KEY, job_name TEXT NOT NULL, status TEXT NOT NULL, -- Running / Paused / CancelRequested / Completed / Failed @@ -536,10 +536,10 @@ CREATE TABLE admin.background_runs ( ); CREATE UNIQUE INDEX one_active_run_per_job - ON admin.background_runs (job_name) + ON jobs.recoverable_runs (job_name) WHERE status IN ('Running', 'Paused', 'CancelRequested'); -CREATE INDEX ON admin.background_runs (last_progress_at) +CREATE INDEX ON jobs.recoverable_runs (last_progress_at) WHERE status = 'Running'; ``` @@ -549,9 +549,9 @@ so it survives concurrent triggers, admin-vs-scheduler races, and transaction interleavings. The `CancelRequested` inclusion prevents a second trigger during cancel from spawning a parallel run. -`admin.*` is a NEW schema — kept distinct from `auth.*` / `storage.*` +`jobs.*` is a NEW schema — kept distinct from `auth.*` / `storage.*` / `admin.*` so operational tables don't pollute domain schemas. Consistency -checks own their own `admin.consistency_findings` in the same +checks own their own `jobs.run_findings` in the same schema. Cursor is `BYTEA`, not JSONB, because per-job cursors are fixed-shape @@ -579,7 +579,7 @@ One `UPDATE` per checkpoint. Cheap, no row-lock contention (this process owns the row): ```sql -UPDATE admin.background_runs +UPDATE jobs.recoverable_runs SET cursor = $2, stats = jsonb_set( stats, @@ -618,7 +618,7 @@ that's a success. Log lines stay meaningful (`outcome=ok`, The engine module exposes: ```rust -pub async fn run_or_resume( +pub async fn run_or_resume( job: Arc, store_factory: &dyn JobStoreFactory, ) -> JobOutcome @@ -671,7 +671,7 @@ At `AppServiceFactory` init, after DB pool is up: ```rust sqlx::query!( - "UPDATE admin.background_runs + "UPDATE jobs.recoverable_runs SET status = 'Paused', error_message = COALESCE(error_message, 'server restart mid-run') WHERE status IN ('Running', 'CancelRequested')" @@ -704,16 +704,16 @@ GET /api/admin/jobs/{name}/runs/{id} ### Native tenants (Part 2) - **Blob storage backend migration.** `migration_job.rs` becomes a - `RecoverableJob` impl. Cursor = last processed blob hash. Retires + `RecoverableJobHandler` impl. Cursor = last processed blob hash. Retires the `Arc>` in-memory struct. - **Reextract audio metadata.** Currently synchronous inside the - admin HTTP request. Becomes a `RecoverableJob` iterating audio + admin HTTP request. Becomes a `RecoverableJobHandler` iterating audio files by `file_id`. - **Reextract image/video capture dates.** Same as above. - **Consistency-check runs.** Every `ConsistencyCheck` impl gets - wrapped by a `RecoverableJob` adapter; the wrapper writes to - `admin.background_runs` via `JobStore`, and separately writes - findings to `admin.consistency_findings` via a check-specific + wrapped by a `RecoverableJobHandler` adapter; the wrapper writes to + `jobs.recoverable_runs` via `JobStore`, and separately writes + findings to `jobs.run_findings` via a check-specific extension trait. See `docs/plan/consistency-check.md`. ### Verification (Part 2) @@ -733,7 +733,7 @@ GET /api/admin/jobs/{name}/runs/{id} 6. **Idempotent replay:** for consistency-check specifically, verify that re-processing the last unpersisted batch does NOT double-record findings (`UNIQUE (run_id, kind, resource_id)` on - `admin.consistency_findings`). + `jobs.run_findings`). 7. **`RunOutcome` bridge log lines:** completed run logs `outcome=ok, extra.completed=true`; paused logs `outcome=ok, extra.paused=true`; failed logs `outcome=err, cause=handler`. @@ -874,7 +874,7 @@ precludes it. ### Job-history observability -`admin.background_runs` already carries the latest run per Part 2 job +`jobs.recoverable_runs` already carries the latest run per Part 2 job — "last run time + status" is a `SELECT DISTINCT ON (job_name) …` query. Deeper history (retention window, per-run drill-down UI) is deferred; the log stream is the source of truth for older runs. @@ -889,7 +889,7 @@ No such need today. - **Cross-job dependencies.** Register-time ordering only, not runtime graph. -- **Retention pruning of terminal `background_runs` rows.** Deferred +- **Retention pruning of terminal `recoverable_runs` rows.** Deferred until the volume warrants a policy. - **Prometheus / OpenMetrics export.** Log-only for now. - **Distributed scheduling.** Single-process. If OxiCloud ever runs