merge: lane/daemon
This commit is contained in:
@@ -0,0 +1,254 @@
|
||||
# ADR 0013 — Task state-machine ownership: CORE, DAEMON, and the shared `paused`
|
||||
|
||||
**Status:** accepted by CORE and PROTO; all four open items resolved · **Date:** 2026-09-09
|
||||
· **Lane:** DAEMON, drafted at PROTO's request (D1 in `core/docs/proto-requests-m1.md`).
|
||||
**CORE's response:** `core/docs/adr-0013-core-response.md` (`lane/core`, commit `c65e664`)
|
||||
— accepted as written, no amendments, plus the pinned `tasks_starved` definition and
|
||||
`pause()`/`resume()` idempotency contract folded in below.
|
||||
**PROTO's response:** the `error`-on-`paused` widening landed as `contracts/` **1.3.0**,
|
||||
`lane/proto` commit `6db304a` — no retype, no new field; only the presence condition on
|
||||
the existing `error: TaskError | null` field widens to include a daemon-initiated pause.
|
||||
Not yet merged to `main` (main is at 1.1.0 as of this writing) — `daemon/src/sched/`'s
|
||||
pause/resume logic should be written against `1.3.0` once that merge lands, not before.
|
||||
|
||||
## Context
|
||||
|
||||
`docs/04` §1 hands CORE the download lifecycle (`probing → connecting → downloading ⇄
|
||||
paused → retry_wait → assembling → verifying → complete | failed`), but `queued` and every
|
||||
scheduler transition are DAEMON's, and neither brief says where the seam is. PROTO raised
|
||||
this as D1 and proposed a split; CORE agreed (`contracts/proto-answers-m1.md` D1); both
|
||||
declined to write it down alone — a two-lane decision recorded by the lane that owns
|
||||
neither half is exactly the failure mode `contracts/README.md` rule 4 exists to prevent.
|
||||
DAEMON drafts it because DAEMON is the one lane touching both halves (it owns the SQL row
|
||||
and the RPC surface; it calls into CORE).
|
||||
|
||||
The wire side is already frozen and does not need to change: `TaskState`
|
||||
(`contracts/schema/types/TaskState.schema.json`) has the twelve values with no field
|
||||
naming who drove a transition, and `event.task.state` carries `previousState` — a client
|
||||
renders what it's told and keeps no state machine of its own. This ADR is about who
|
||||
*decides* a transition, not the wire shape of one.
|
||||
|
||||
This is also where ADR 0011 (admission control) and the state machine meet: `queued →
|
||||
connecting` is the admission event, and `retry_wait` looks identical to starvation from
|
||||
inside the segment-budget accounting. Both are addressed below because getting them wrong
|
||||
independently produces the same false alarm from two different directions.
|
||||
|
||||
## Decision
|
||||
|
||||
### 1. Ownership, by state
|
||||
|
||||
| State | Driven by | Entered from | Notes |
|
||||
|---|---|---|---|
|
||||
| `new` | DAEMON | (creation) | `download.add` persists the row before CORE knows the task exists. |
|
||||
| `queued` | DAEMON | `new`, `paused` (resume, subject to §3) | Scheduler admission pool. CORE has no concept of "queued" — `retry_wait` re-enters `connecting` directly inside CORE and never passes back through `queued`. |
|
||||
| `probing` | CORE | `queued` | DAEMON calls CORE's `start()` once admitted (ADR 0011 §1); CORE owns everything from here until it hands back a terminal or `paused` state. |
|
||||
| `connecting` | CORE | `probing`, `retry_wait` | |
|
||||
| `downloading` | CORE | `connecting` | |
|
||||
| `paused` | **shared** | any CORE state, `queued` | See §2 — this is the one state either side may enter unilaterally. |
|
||||
| `retry_wait` | CORE | `connecting`, `downloading` | CORE's own backoff timer (`docs/04` §7); DAEMON does not schedule retries. |
|
||||
| `assembling` | CORE | `downloading` | Normally a no-op rename; a real state for HLS/DASH muxing. |
|
||||
| `verifying` | CORE | `assembling` | Checksum verification when requested. |
|
||||
| `complete` | CORE | `verifying` | Terminal. |
|
||||
| `failed` | CORE | any CORE state | Terminal. `max_retries_exhausted` after `retry_wait`, or any non-retryable error. |
|
||||
| `cancelled` | DAEMON | any state | Terminal. Always user- or policy-initiated (`download.cancel`, category/queue removal) — CORE never cancels on its own; it only executes the teardown DAEMON asked for and confirms. |
|
||||
|
||||
Reading the table as a picture:
|
||||
|
||||
```
|
||||
DAEMON: new ──▶ queued ──▶[admit]──▶ (hand to CORE)
|
||||
▲ │
|
||||
│ resume CORE: probing ─▶ connecting ─▶ downloading
|
||||
pause────┤ │ │ ▲ │ │
|
||||
(either side)│ retry_wait◀────────┘ │ │ │
|
||||
▼ │ (backoff done)│ ▼ ▼
|
||||
paused ◀──────────(auto-pause)─────┘ assembling paused
|
||||
│ (auto)
|
||||
verifying
|
||||
│
|
||||
complete
|
||||
(from anywhere, DAEMON-driven) ──────────────────────▶ cancelled
|
||||
(from any CORE state, CORE-driven, non-retryable) ───▶ failed
|
||||
```
|
||||
|
||||
### 2. `paused` is shared, and idempotency is the whole contract
|
||||
|
||||
Both sides can put a task in `paused`, for disjoint reasons:
|
||||
|
||||
- **DAEMON-initiated**: user clicks pause, a schedule window closes, a queue is stopped,
|
||||
`Queue.onComplete`, or the admission governor reconciling a lowered `maxActiveSegments`
|
||||
(ADR 0011 §2 — pausing the lowest-priority excess). DAEMON calls CORE's `pause(TaskId)`.
|
||||
- **CORE-initiated ("auto-pause")**: `auth_required` (401/407, `docs/04` §7),
|
||||
`server_file_changed` (F1's carrier, `contracts/proto-answers-m1.md`), disk full. CORE
|
||||
transitions to `paused` on its own and reports it up through the existing
|
||||
state-change callback — the same path every other CORE-driven transition uses. This is
|
||||
not a new mechanism.
|
||||
|
||||
The contract that makes this safe, per CORE's sign-off (`core/docs/adr-0013-core-response.md`):
|
||||
|
||||
- **`pause(TaskId)` is idempotent: a no-op success** on a task already `paused` (however
|
||||
it got there), and **also a no-op success on a terminal task** (`complete`/`failed`/
|
||||
`cancelled`) — a pause racing a completion is not an error. **`resume(TaskId)` is a
|
||||
no-op success on a task that is not paused.** The only error either returns is
|
||||
`task_not_found`. CORE emits no state-change event for a no-op call — DAEMON reads the
|
||||
resulting state from the normal state-change callback / `TaskDetail`, never from
|
||||
`pause()`'s or `resume()`'s return value. If CORE is mid-transition into `paused` (its
|
||||
own auto-pause) when DAEMON's `pause()` arrives, the task ends up `paused` once, with
|
||||
exactly one state-change event.
|
||||
- **DAEMON persists *why* a task is paused**, in the `tasks` table, not in `TaskState`
|
||||
itself (the wire type stays a flat enum — this is DAEMON-local bookkeeping, not a
|
||||
contract change). A `pauseReason` distinguishing at least `user`, `schedule`,
|
||||
`queue_stopped`, `admission_reconcile`, and CORE's `error.code` when auto-paused —
|
||||
now readable off `event.task.state.error` since PROTO's 1.3.0 widening (see "A
|
||||
contract gap this ADR surfaced, now closed" below). This is what makes §3's resume
|
||||
rule possible.
|
||||
- **CORE does not need to track why it's paused past the current occurrence.** Once
|
||||
paused, CORE's job is done; DAEMON is the only side that later decides whether to
|
||||
resume, and DAEMON is also the only side with persistent storage to remember the
|
||||
reason across a restart.
|
||||
|
||||
### 3. Resume must not cross reasons
|
||||
|
||||
The bug this section exists to prevent: a schedule window opens, DAEMON blindly resumes
|
||||
every `paused` task in that queue, and it resumes a task CORE paused because it's waiting
|
||||
on credentials that were never provided. The task immediately re-fails or re-pauses, looks
|
||||
like a flapping bug, and burns a retry.
|
||||
|
||||
Rule: **DAEMON resumes a task only when the reason it recorded matches the event that
|
||||
justifies resuming.** A schedule window opening resumes tasks paused for `schedule`. A
|
||||
user clicking "resume" resumes anything (explicit user intent overrides any reason).
|
||||
`auth_required` and `server_file_changed` are resumed only by the paths that actually
|
||||
address them — `download.provideAuth` (F2) and the user's restart-decision response (F1) —
|
||||
never by the scheduler. This means `queued` is not the only state a schedule can put a
|
||||
task back into a run cycle from; the scheduler's "should this task be running right now"
|
||||
check must skip tasks paused for a reason it doesn't own.
|
||||
|
||||
### 4. `retry_wait` is not starvation
|
||||
|
||||
ADR 0011 §3.6 defines a starved task as a running task with `segments_active(id) == 0`,
|
||||
and asserts `tasks_starved == 0` in steady state past a bounded delay. A task in
|
||||
`retry_wait` also holds zero segments, deliberately, for up to 60 s (`docs/04` §7's backoff
|
||||
cap) — and it is **not** admission-starved, it is CORE's own policy holding it idle.
|
||||
|
||||
**Pinned by CORE's sign-off**, tightening ADR 0011's definition rather than special-casing
|
||||
it: `tasks_starved` counts only tasks whose `TaskState` is `connecting` or `downloading`
|
||||
**and** `segments_active == 0` — precisely "the allocator has not granted a slot to a task
|
||||
that is asking for one."
|
||||
|
||||
| Task state | In `tasks_starved`? | Why |
|
||||
|---|---|---|
|
||||
| `probing` | no | uses the probe pool (ADR 0011 §5), not the segment budget |
|
||||
| `connecting`, 0 segments | **yes** | admitted + probed, waiting on the allocator's first grant — the real starvation case |
|
||||
| `connecting`/`downloading`, ≥1 segment | no | a segment in its own `connecting` sub-state counts as held (ADR 0011 amendment A3) |
|
||||
| `downloading`, 0 segments | **yes** | held slots and lost them all (e.g. every segment failed and is being re-requested) — transient, still real |
|
||||
| `retry_wait` | no | CORE's backoff timer holds it at zero *deliberately*; not asking the allocator for anything until it re-enters `connecting` |
|
||||
| `paused` (either-initiated) | no | not asking for a slot |
|
||||
| `new`, `queued`, `assembling`, `verifying`, terminal | no | outside the counted state set |
|
||||
|
||||
So `retry_wait` and auto-paused tasks are excluded **structurally, by not being in the
|
||||
`{connecting, downloading}` state set** — not via a special case that could rot as the
|
||||
engine evolves. `starved_tasks()` returns exactly the TaskIds in this count;
|
||||
`starved_since(id)` is defined only for them. A task in `retry_wait` still counts against
|
||||
`connection.maxConcurrentDownloads` from DAEMON's side (it is running, not requeued) but
|
||||
contributes nothing to the segment allocator's guarantee pass until it re-enters
|
||||
`connecting`.
|
||||
|
||||
### 5. Restart — CORE holds no persistent state, DAEMON reloads to `queued`
|
||||
|
||||
Per the M1 DoD ("kill and restart the daemon mid-download: all tasks reload with correct
|
||||
state and resume") and the layering rule (CORE has no SQL, no state survives a CORE
|
||||
restart except what's on disk in `.veloxpart.meta`, which CORE reads back itself): on
|
||||
daemon startup, DAEMON loads every non-terminal task from SQLite. Any task whose persisted
|
||||
`TaskState` was a CORE-owned state (`probing` through `verifying`) is **rewritten to
|
||||
`queued`** in memory before the scheduler sees it — the on-disk `TaskState` is a
|
||||
last-known-value, not a resumable position, because CORE's in-process state died with the
|
||||
process. The scheduler re-admits it exactly like any other queued task; CORE re-derives
|
||||
where to actually resume from `.veloxpart.meta` and re-validates with `If-Range`
|
||||
(`docs/04` §5), independent of what SQL said the state was.
|
||||
|
||||
`paused` tasks reload as `paused`, with their `pauseReason` intact, and are not
|
||||
auto-admitted — §3 applies identically after a restart as it does live.
|
||||
|
||||
**Confirmed by CORE, fully.** `start(TaskId)` transparently checks for a valid
|
||||
`.veloxpart.meta` sidecar (CORE's stage 5), re-validates with `If-Range` (`docs/04` §5),
|
||||
and either resumes from the recorded offsets or restarts if the sidecar is missing or
|
||||
fails validation. DAEMON does nothing special beyond rewriting CORE-owned states to
|
||||
`queued` and re-admitting, as this section already said.
|
||||
|
||||
**Two notes from CORE, not objections to the ADR:** the *transition* into `paused` is
|
||||
honoured from any CORE state per §1's table, but the *work interruption* is best-effort —
|
||||
a pause during `verifying` discards the in-progress hash and re-hashes from the start on
|
||||
resume (cheap, bounded); a pause during `assembling` (HLS/DASH mux) is an M4 concern and
|
||||
may not be cleanly interruptible mid-mux. Neither changes this ADR's API shape.
|
||||
|
||||
## Consequences
|
||||
|
||||
- `daemon/src/sched/` calls `start(TaskId)` exactly once per admission (`queued →
|
||||
probing`), never re-enters a CORE-owned state directly, and treats every CORE-owned
|
||||
state as opaque past that call except for reading it back for projection.
|
||||
- The `tasks` table needs a `pauseReason` column (DAEMON-local; not a wire type) before
|
||||
`sched/`'s pause/resume logic can be written correctly — flagging as a concrete
|
||||
follow-up, not blocking this ADR's acceptance.
|
||||
- CORE's `starved_tasks()` (ADR 0011) excludes `retry_wait` and CORE-auto-paused tasks by
|
||||
construction — pinned in CORE's sign-off as the `{connecting, downloading} ∧
|
||||
segments_active == 0` definition in §4's table — so DAEMON's governor-invariant warning
|
||||
will not false-positive on a normal backoff cycle.
|
||||
- No `contracts/` change. `TaskState`, `event.task.state`, and `previousState` are already
|
||||
sufficient; this ADR is entirely about which process calls which function when.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**A single owner drives every transition (CORE, told about queues).** Rejected — this is
|
||||
the D1 problem restated with CORE holding the SQL-shaped concepts (`queued`, schedules,
|
||||
priority) that the layering rule (`CLAUDE.md` §3) forbids it from touching.
|
||||
|
||||
**DAEMON drives every transition, treating CORE as a dumb byte-mover.** Rejected — CORE's
|
||||
internal states (`retry_wait`, `assembling`, `verifying`) depend on engine internals
|
||||
(backoff timers, mux completion, streaming hash state) DAEMON has no visibility into
|
||||
without CORE reporting them; forcing DAEMON to poll or reimplement that timing duplicates
|
||||
`docs/04` §7 in two places and they will drift.
|
||||
|
||||
**A `pauseReason` on the wire (`TaskState` split into `paused_user` / `paused_auto` /
|
||||
etc.).** Rejected — it roughly doubles the enum for a fact only DAEMON's resume logic
|
||||
needs, and adding wire cardinality for internal bookkeeping is the kind of thing that
|
||||
becomes a compatibility problem the moment a client starts branching on it. The value
|
||||
DAEMON needs (CORE's reason) travels on the existing `error` field instead — see the
|
||||
closed contract gap below — without touching `TaskState` itself.
|
||||
|
||||
## A contract gap this ADR surfaced, now closed
|
||||
|
||||
`event.task.state.schema.json`'s description scoped `error` to "whenever the new state is
|
||||
failed or retry_wait" — **not** `paused`, at the time this ADR was drafted, and the one
|
||||
fixture (`event.task.state.json`) only exercised the `failed` case. So CORE auto-pausing
|
||||
for `auth_required` or `server_file_changed` had no wire signal telling DAEMON *why* — §2/
|
||||
§3 of this ADR were unbuildable without one.
|
||||
|
||||
**Closed by PROTO**, `contracts/` 1.3.0 (`lane/proto` commit `6db304a`, see the status
|
||||
line at top for the merge caveat): minor widening of the existing field's presence
|
||||
condition, per `contracts/README.md` rule 4 — no new field, no retype, `error` stays
|
||||
`TaskError | null`. It is now populated on a `paused` transition whenever CORE entered it
|
||||
unilaterally; `error: null` on a DAEMON-initiated pause is unchanged. New fixture
|
||||
`event.task.state.auto-paused.json` exercises the auto-pause case; the existing
|
||||
`event.task.state.json` fixture's stale "exactly when failed or retry_wait" assertion was
|
||||
corrected in the same change.
|
||||
|
||||
## Resolution of the four open items
|
||||
|
||||
1. **`starved_tasks()` excludes `retry_wait`/auto-paused by construction — confirmed.**
|
||||
Pinned as a design commitment in CORE's sign-off (§4's table above): the counted set is
|
||||
`{connecting, downloading} ∧ segments_active == 0`, so exclusion is structural, not a
|
||||
special case.
|
||||
2. **`pause()`/`resume()` idempotency — confirmed as specified in §2 above**, plus two
|
||||
details this ADR hadn't anticipated: a no-op call also succeeds against a *terminal*
|
||||
task (pause racing completion isn't an error), and no state-change event fires for a
|
||||
no-op — DAEMON reads resulting state from the callback/`TaskDetail`, never from the
|
||||
call's return value.
|
||||
3. **Landed — PROTO, `contracts/` 1.3.0** (`lane/proto` commit `6db304a`, not yet merged
|
||||
to `main`). `event.task.state.error` and `TaskSummary.error` are now populated "on
|
||||
every `failed` or `retry_wait` transition, and on a `paused` transition the daemon
|
||||
entered unilaterally" — a deliberate pause still carries `error: null`. New fixture
|
||||
`event.task.state.auto-paused.json` exercises an `auth_required` auto-pause directly.
|
||||
DAEMON's §3 resume rule can be implemented once `sched/` is built against `main` at
|
||||
1.3.0 or later.
|
||||
4. **CORE adopts "auto-pause".** No new wire or API term — the discriminator stays
|
||||
`state == paused` plus the presence of an `Error` (present ⇒ CORE-initiated, absent ⇒
|
||||
DAEMON-initiated), exactly as §2 already specified.
|
||||
Reference in New Issue
Block a user