diff --git a/daemon/docs/core-requests-m1.md b/daemon/docs/core-requests-m1.md new file mode 100644 index 0000000..6a2e06f --- /dev/null +++ b/daemon/docs/core-requests-m1.md @@ -0,0 +1,67 @@ +# DAEMON → CORE — the engine API `sched/` needs before it can be written + +Status: **resolved**. CORE answered in full: `core/docs/adr-0011-core-response.md` +(`lane/core`, commit `7bf5cb5`). Kept as a record of what was asked and the shape of the +answer; the API itself is now specified in +`docs/adr/0011-admission-control-and-the-segment-budget.md` §"Engine API `sched/` is built +against". `sched/` may be written against it. + +Ranking followed `contracts/README.md` rule 4 conventions even though this wasn't a +`contracts/` change: new API surface = cheap, land anytime; a behavioural promise (min-1 +fairness) = needed CORE's explicit sign-off before DAEMON built on the assumption. That +sign-off is in. + +--- + +## C1. Occupancy read-out, not inference — **resolved** + +Requested `budget()`, `segments_active(TaskId)`, `on_budget_changed`. CORE's answer adds +`starved_tasks()` and `starved_since(TaskId)` (amendment A3) and pins two definitions: +`segments_active(id)` counts a segment in `connecting` state as held (it is progress, not +starvation), and `tasks_starved` counts only `segments_active == 0`. See ADR 0011 §3.6. + +## C2. `set_max_active_segments(uint32_t)` live-apply — **resolved: drain, never kill** + +Confirmed DAEMON's assumption. Lowering runs in-flight segments to their next boundary; no +new segment starts while over the new ceiling; nothing is aborted, no partial range lost. +If the new ceiling is below the running-task count, CORE honours min-1 for the top-priority +subset and reports the rest via `tasks_starved` — DAEMON's governor must reconcile and +pause the lowest-priority excess itself (CORE does not auto-pause). See ADR 0011 §2. + +## C3. `set_host_segment_cap(host, uint32_t)` — **resolved, confirmed as proposed** + +CORE keeps the `host → cap` map and derives a task's host from its URL/mirror set; DAEMON +owns the table and pushes it. See ADR 0011 §4. + +## C4. Contract gap: `connection.maxActiveSegments` — **resolved by PROTO** + +PROTO landed it (ADR 0012, `connection.maxActiveSegments` default 32, +`connection.maxTotalBufferBytes` default 128 MiB, `TaskDetail.effectiveBufferBytes`) while +this was in flight. No daemon-local stopgap needed — `sched/` reads the wire field +directly. See ADR 0011 §6. + +## C5. Fairness rule sign-off — **resolved, with two amendments** + +CORE confirmed min-1-before-seconds is implementable without a priority-inversion at slot +release (two-pass allocator: guarantee pass over zero-slot tasks in DAEMON's priority +order, then a growth pass; a released slot always re-enters the pool at pass 1, never +handed back locally). Two amendments to what DAEMON assumed: + +- **A1** — "steal" (slot-neutral, unchanged) isn't the whole mechanism; **"yield"** is the + slot-transfer operation that actually satisfies min-1 out of a full budget: an + over-quota task releases one slot at its next segment boundary, bounded by that + segment's remaining bytes. +- **A2** — "admission implies progress" is **bounded-delay**, not immediate: + `time_to_first_slot ≤ min(next yield boundary, low_speed_secs) + connect_timeout`, not + "connect timeout + per-host cap" alone. DAEMON's starvation-invariant assertion window + widened from the originally proposed 2 s to `low_speed_secs + connect_timeout` (~45 s) + accordingly. + +Priority order (open item 4) is an ordered `TaskId` list pushed via `set_task_order` on +change — not an integer, not per-tick. See ADR 0011 §3. + +## C6. Probe pool sized outside the segment budget — **resolved, confirmed** + +Dedicated pool, default size 4, `set_probe_pool_size(uint32_t)`, independent of +`maxActiveSegments`; probe cancellation is immediate. DAEMON still bounds probe +*submission* on its own side. See ADR 0011 §5. diff --git a/docs/adr/0011-admission-control-and-the-segment-budget.md b/docs/adr/0011-admission-control-and-the-segment-budget.md new file mode 100644 index 0000000..0002e1e --- /dev/null +++ b/docs/adr/0011-admission-control-and-the-segment-budget.md @@ -0,0 +1,247 @@ +# ADR 0011 — Admission control, the segment budget, and who counts what + +**Status:** accepted · **Date:** 2026-09-09 · **Lane:** DAEMON, signed off by CORE +**Companion request:** `daemon/docs/core-requests-m1.md` (the engine API this depends on) +**CORE's response:** `core/docs/adr-0011-core-response.md` (`lane/core`, commit `7bf5cb5`) +— accepted with three amendments (A1–A3, folded in below) and answers to all five open +questions. `daemon/src/sched/` is unblocked. + +## Context + +Two lanes were each building a global concurrency governor, neither brief mentioned the +other, and they were counting different things. + +* **CORE** added `connection.maxActiveSegments` (default 32, landed on the wire by + PROTO's ADR 0012) inside the engine — a ceiling on segments actually transferring at + once. It is the mechanism that makes the RSS target in `docs/04` §8 hold + (`core/docs/buffer-sizing.md`), so it is not optional. +* **DAEMON** is briefed to build "a concurrency governor (global max active, per-queue max, + per-host caps)", backed by `connection.maxConcurrentDownloads`, `Queue.maxConcurrent`, + schedules and queue order. + +Left alone this lands in one of two states, and both are bad in a way that is hard to +diagnose after the fact: + +1. **Double-throttling.** Both lanes enforce a global ceiling, so the effective limit is + the minimum of two numbers the user set independently. The link runs at half rate and + it reads as a performance bug in the engine, not as a policy collision. +2. **The gap.** Each lane assumes the other holds the line. Nobody does, 20 downloads open + 160 connections, and the RSS budget that `maxActiveSegments` exists to defend is gone. + +There is a third problem underneath both. With `maxActiveSegments = 32` and +`connection.maxSegmentsPerDownload` capped at 32, one download can hold the entire segment +budget. If the daemon admits a second task and the engine has no slot to give it, the task +is *running* and transferring nothing: every DAEMON assumption that admission implies +progress — stall detection, speed accounting, queue drain, "when queue completes" — is +then wrong. CORE owns that fairness rule. DAEMON cannot write a governor without knowing +what it is. + +## Decision + +### 1. Every ceiling is enforced exactly once, by the lane that owns the unit it counts + +This is the whole ADR in one line. The two governors stay two governors, on two axes, with +strictly non-overlapping units: + +| Ceiling | Unit | Enforced by | Configured by | +|---|---|---|---| +| `connection.maxConcurrentDownloads` | tasks | DAEMON | user | +| `Queue.maxConcurrent` | tasks | DAEMON | user | +| per-host **task** cap | tasks | DAEMON | host table | +| schedules, windows, queue order, priority | tasks | DAEMON | user | +| `connection.maxActiveSegments` | segments | **CORE** | user | +| `connection.maxSegmentsPerDownload` | segments | **CORE** | user, per task | +| per-host **segment** cap | segments | **CORE** | host table, pushed by DAEMON | +| `bufferBytes` / `maxTotalBufferBytes` | bytes | **CORE** | user | +| speed limits (global → queue → task) | bytes/s | **CORE** | user, pushed by DAEMON | + +Corollaries, and these are the parts that actually prevent the two failure modes: + +* **DAEMON never counts segments to make an admission decision.** Not directly, and not by + inferring occupancy from a download count. Its governor sees tasks. +* **CORE never refuses admission.** `start()` always accepts. The engine paces the task + inside the segment budget; it does not decide that the task should not be running. A + refusal would be a policy decision, and policy lives in the daemon with the queues and + the SQL behind it. +* The pattern generalises: **DAEMON decides policy and configures; CORE enforces every + ceiling counted in engine-internal units.** Rate limiting (`docs/04` §6) already works + this way. Recording it here so it is not re-litigated per subsystem. + +Signed off by CORE as written, including the shared-semaphore rejection in "Alternatives +considered" below. + +### 2. The one legitimate coupling — a clamp, not a second enforcement + +DAEMON reads `maxActiveSegments` in exactly one place: + +``` +effective_max_running_tasks = min(connection.maxConcurrentDownloads, + connection.maxActiveSegments) +``` + +with the same clamp applied per queue against that queue's share. The purpose is narrow: +never admit more concurrently-running tasks than the segment budget can give one segment +each. It is expressed in tasks, it throttles nothing that CORE also throttles, and it is +the reason §3's fairness rule is satisfiable — **CORE's Q1 answer is explicit that min-1 +liveness depends on DAEMON honouring this clamp.** Admitting 3 running tasks against a +budget of 2 starves one by construction and no fairness rule on CORE's side fixes it. + +At the shipped defaults (`maxConcurrentDownloads` 5, `maxActiveSegments` 32) the clamp is +not binding. It binds when a user raises concurrency to 64 or lowers the segment budget. + +**After a live lowering of `maxActiveSegments`** below the running-task count, CORE +honours min-1 for the top `new_ceiling` tasks in DAEMON's priority order and reports the +rest in `tasks_starved` — it does **not** auto-pause them (policy stays with DAEMON). The +governor must reconcile its running set against the new clamp on every +`on_budget_changed` delivery and pause the lowest-priority excess itself. + +### 3. CORE's fairness rule — what DAEMON is allowed to assume + +CORE owns this; the mechanism below is CORE's, confirmed in its ADR 0011 response. + +1. **Min-1 before seconds.** No task receives a second segment slot while any admitted task + holds zero. A task's first slot always outranks another task's growth. Implemented as + two ordered passes re-run on every budget-changing edge: a **guarantee pass** over + zero-slot tasks in DAEMON's priority order, then a **growth pass** round-robining + remaining budget over running tasks up to their effective per-task cap. A released slot + always re-enters the pool and re-runs pass 1 from the top — it is never handed back + directly to the releasing task — so a task that drops to zero re-enters the guarantee + queue at its priority position, not the back. No inversion at slot release. +2. Beyond the first slot, remaining budget is distributed round-robin over running tasks in + the priority order DAEMON supplies (`set_task_order`), up to each task's effective + per-task cap: `min(spec.segments ?? maxSegmentsPerDownload, per-host segment cap, 1 if + not resumable)`. +3. Slots are released on segment completion, pause, and failure, by two distinct + operations: + - **Steal** — a worker that finished its range takes the tail of the largest remaining + range. Same worker, same slot: slot-neutral, exactly as first described. + - **Yield** — the mechanism that actually satisfies rule 1 when the budget is full: the + allocator marks an over-quota task to release one slot *at its next segment + boundary*, bounded by that segment's remaining bytes. Never a mid-segment kill. This + is a slot **transfer**, not slot-neutral — CORE's amendment A1, since "steal" alone + doesn't explain how a starved task ever gets its first slot out of a full budget. +4. A paused task holds no slots. +5. **Admission implies progress, but the bound is delay, not immediacy.** When the budget + is full of healthy incumbents, a newly-admitted task's first slot appears at the next + yield boundary, hard-capped by the stall timeout: + ``` + time_to_first_slot ≤ min(incumbent's next segment boundary, low_speed_secs) + connect_timeout + ``` + not "connect timeout + per-host cap" alone, as originally assumed. (CORE amendment A2; + a future preemptive-split optimization — truncating an incumbent's range ahead of its + current offset — is on the table post-M1 if the yield delay proves painful in soak + testing, but doesn't change this API.) +6. **Invariant:** `budget().tasks_starved == 0` in steady state, where a task counts as + starved only if `segments_active(id) == 0` — a segment in `connecting` state is progress, + not starvation (CORE amendment A3). DAEMON asserts this and, if it observes a non-zero + value persisting past **`low_speed_secs + connect_timeout` (~45 s, not the originally + proposed 2 s** — CORE's A2 correction, since a legitimately full budget with a slow + incumbent tail can hold a new task at zero that long with nothing actually wrong), logs + a governor-invariant warning and surfaces it in `velox ls --json` using + `starved_since(TaskId)` to show how long. It does **not** compensate by throttling + admission — compensating is how the two governors would silently grow back into one. + +Consequence for the starvation question in the brief: one download **cannot** permanently +take the whole budget away from the next, because rule 1 makes the next task's first slot +outrank the incumbent's second via yield. A single download alone in the system does +legitimately grow to 32 segments, and gives slots back (bounded-delay, per rule 5) as +tasks arrive — growth is opportunistic, the first slot is guaranteed within a bounded time. + +### 4. Per-host caps are split by unit, from one table + +Both briefs say "per-host caps" and they are not the same cap. + +* CORE enforces per-host **segment** caps — it owns the connections and is the only place + segments are counted (`docs/04` §3, "clamped per-host by settings"). CORE derives a + task's host from its URL and mirror set; DAEMON does not need to push per-task host + resolution, only the cap table. +* DAEMON enforces a per-host **task** cap, set to that host's segment cap, so it can never + admit more tasks for one host than that host can be given one segment each. Without this, + rule 3.1 is unsatisfiable: four tasks on a host capped at 4 connections is fine, five is + a guaranteed starved task no fairness rule can fix. +* The table itself is DAEMON state (SQLite, `settings`), pushed into the engine via + `set_host_segment_cap(std::string host, uint32_t)`. One source of truth, two enforcement + points, different units. + +### 5. Probes do not consume segment slots + +A `download.probe` is a HEAD or a one-byte ranged GET. Charging it against the segment +budget would let a burst of probes starve transfers, and probes are on the latency path for +`capture.offer`'s 750 ms deadline. CORE bounds concurrent probes with its own dedicated +pool, default size 4, `set_probe_pool_size(uint32_t)`, entirely independent of the segment +budget — confirmed by CORE. Probe cancellation is immediate, so `capture.offer` can answer +`ignore` first and probe after with no risk of blocking on a stuck probe. CORE bounds probe +*concurrency*; DAEMON still bounds probe *submission* on its own side (queue depth is a +DAEMON policy question, not an engine one). + +### 6. `connection.maxActiveSegments` is now on the wire (PROTO ADR 0012) + +Resolved: PROTO landed `connection.maxActiveSegments` (default 32) and +`connection.maxTotalBufferBytes` (default 128 MiB) in `Settings.schema.json`/`SettingKey` +in ADR 0012, alongside `TaskDetail.effectiveBufferBytes`. DAEMON reads and writes it +through `settings.get`/`settings.set` like any other connection setting — no daemon-local +stopgap needed. `sched/` can reference the wire field directly. + +## Alternatives considered + +**DAEMON enforces both.** The governor would have to predict each task's effective segment +count to spend a segment budget in task units — but that count depends on the probe result, +the per-host cap, the resumability demotion and live steals, all engine-internal and all +changing continuously. Predicting it means either over-admitting (the gap) or leaving the +link idle. Rejected: it asks the daemon to model the engine. + +**CORE enforces both.** The engine would take every task and decide which run. That drags +queues, schedules, priority, and "when queue completes" into `core/`, which the layering +rule forbids and which would need SQL to be correct. Rejected. + +**A shared semaphore object handed to both lanes.** Superficially the "one counter" answer, +but it makes a mutable engine resource part of the daemon's API surface, inverts the +dependency direction, and is the first thing that will deadlock under pause-during-steal. +Rejected: one counter, one owner, read-only snapshots for everyone else. + +## Consequences + +* `daemon/src/sched/` may be written against a task-unit model only. A segment count + appearing in an admission decision is a review-blocking defect. +* DAEMON reads occupancy through the engine API below rather than inferring it — + needed to project `TaskSummary.segments` (ADR 0010: the *effective* count) without + guessing. +* CORE's fairness rule has a test DAEMON can point at: N tasks admitted, budget smaller + than N × their per-task caps, assert every task reaches ≥ 1 segment within + `low_speed_secs + connect_timeout`, and `tasks_starved == 0` in steady state thereafter. +* The governor must reconcile its running set on every `on_budget_changed` delivery, + pausing the lowest-priority excess when a live lowering of `maxActiveSegments` leaves + some running tasks permanently below `new_ceiling` (§2). + +## Engine API `sched/` is built against (CORE, `core/docs/adr-0011-core-response.md`) + +``` +void set_max_active_segments(uint32_t); // drain-not-kill (§2, §3.5) +void set_host_segment_cap(std::string host, uint32_t); // §4 +void set_task_order(std::span); // pushed on change, not per tick +void set_probe_pool_size(uint32_t); // default 4, §5 + +struct EngineBudget { uint32_t total; uint32_t active; uint32_t tasks_starved; }; +EngineBudget budget() const; +uint32_t segments_active(TaskId) const; // includes `connecting` +std::vector starved_tasks() const; +std::optional starved_since(TaskId) const; +void on_budget_changed(std::function); // 4 Hz + immediate on the + // tasks_starved 0↔nonzero edge +``` + +All of it lands with CORE's stage 6 (segmenter/stealer) / stage 8 (download_task) — not on +the M1 critical path ahead of where `sched/` needs it, per CORE. + +## Resolved questions (were open, now answered by CORE) + +1. **Min-1 without priority inversion at slot release — yes**, per §3.1 above. +2. **Probe pool outside the segment budget, size 4 — confirmed**, §5. +3. **Live `set_max_active_segments()` — drain, never kill**, §2/§3.5. +4. **Priority shape — an ordered `TaskId` list, pushed on change**, not an integer and not + per-tick. DAEMON already owns the total order (queue precedence, admission-time + tie-break); pushing an integer would force CORE to reimplement tie-breaking, which is + DAEMON policy. +5. **Budget-change callback — 4 Hz coalesced, plus an immediate fire on the + `tasks_starved` 0↔non-zero transition** so the steady-state invariant check and any UI + reaction aren't lagged by up to 250 ms.