From ecedfac903b0329a89c6b2d1188401e217a78c2b Mon Sep 17 00:00:00 2001 From: sami Date: Wed, 9 Sep 2026 23:18:07 +0400 Subject: [PATCH 1/2] =?UTF-8?q?daemon:=20propose=20ADR=200011=20=E2=80=94?= =?UTF-8?q?=20admission=20control=20vs.=20the=20segment=20budget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settles the open interface question in AGENT-DAEMON.md before sched/ is written: DAEMON's concurrency governor (global/per-queue/per-host, task units) and CORE's maxActiveSegments (segment units) are two governors on two axes with non-overlapping enforcement — each lane enforces exactly the ceilings counted in the units it owns, with one narrow task-unit clamp against maxActiveSegments. Records the fairness rule DAEMON needs from CORE (min-1-before-seconds) so admission implies progress even when one download could otherwise hold the entire segment budget. Companion daemon/docs/core-requests-m1.md is the concrete engine API ask (budget()/segments_active()/on_budget_changed, live-apply semantics for set_max_active_segments, set_host_segment_cap, probe pool sizing) plus one contract gap for PROTO (connection.maxActiveSegments missing from Settings.schema.json). Status: proposed, pending CORE sign-off on the five open items at the end of the ADR. daemon/src/sched/ does not land until that lands. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/docs/core-requests-m1.md | 78 ++++++++ ...dmission-control-and-the-segment-budget.md | 182 ++++++++++++++++++ 2 files changed, 260 insertions(+) create mode 100644 daemon/docs/core-requests-m1.md create mode 100644 docs/adr/0011-admission-control-and-the-segment-budget.md diff --git a/daemon/docs/core-requests-m1.md b/daemon/docs/core-requests-m1.md new file mode 100644 index 0000000..1d3ebf8 --- /dev/null +++ b/daemon/docs/core-requests-m1.md @@ -0,0 +1,78 @@ +# DAEMON → CORE — the engine API `sched/` needs before it can be written + +Status: **open**. Companion to `docs/adr/0011-admission-control-and-the-segment-budget.md`, +which settles *who* enforces what. This doc is the concrete API ask that ADR depends on. +Also carries one contract gap (C4) that is PROTO's, not CORE's, filed here because it was +found while writing the ADR. + +Ranking follows `contracts/README.md` rule 4 conventions even though this isn't a +`contracts/` change: new API surface = cheap, land anytime; a behavioural promise (min-1 +fairness) = needs CORE's explicit sign-off before DAEMON builds on the assumption. + +--- + +## C1. Occupancy read-out, not inference + +DAEMON's governor (ADR 0011 §1) must never count segments to make an admission decision, +which means it needs to *read* engine occupancy rather than derive it from how many tasks +it thinks it started. Requesting: + +- `EngineBudget budget() const` — `{ total: uint32, active: uint32, tasks_starved: uint32 }` + where `tasks_starved` is the count of running tasks currently holding zero segments + (ADR 0011 §3.6 — DAEMON asserts this is 0 in steady state and logs if it isn't). +- `uint32_t segments_active(TaskId) const` — for `TaskSummary.segments` projection + (ADR 0010: the *effective* count) and for `velox ls --json`. +- A coalesced `on_budget_changed(callback)`, batched at the same ≤4 Hz as + `event.task.progress` (brief item 5) — DAEMON is not polling this on a tighter loop. + +## C2. `set_max_active_segments(uint32_t)` and its live-apply semantics + +Brief already has this as a settings value CORE enforces. DAEMON needs to know: does +lowering it live drain existing segments down to the new ceiling (segments finish, no new +ones start until under budget), or does it kill in-flight segments? DAEMON's assumption, +pending your answer, is drain-not-kill — a live cut to 8 must not abort 24 in-flight +segments and lose their partial ranges. ADR 0011 open item 3. + +## C3. `set_host_segment_cap(host, uint32_t)` + +The per-host table is DAEMON state (SQLite `settings`/a host-cap table); CORE enforces it +in segment units. Requesting a push API so there is one source of truth for the table and +two enforcement points, per ADR 0011 §4. DAEMON derives its own per-host **task** cap from +the same table value — same number, different unit, pushed once. + +## C4. Contract gap: `connection.maxActiveSegments` isn't in `Settings.schema.json` + +Not CORE's item — flagging because ADR 0011 depends on it and `core/docs/buffer-sizing.md` +already asked for it. `SettingKey` is a closed enum (`additionalProperties: false`), so +DAEMON cannot expose this key through `settings.get`/`settings.set` until PROTO lands it. +Until then DAEMON holds the value (default 32) locally and passes it to +`set_max_active_segments()` at startup only — no wire exposure, so `Options → Connection` +can't show or set it yet. Same ask as CORE's B2a bundle; landing them together is fine. + +## C5. Fairness rule sign-off (ADR 0011 §3) + +Not new API — a behavioural contract. DAEMON's governor is built assuming: + +1. Min-1 before seconds: no running task gets a second slot while another running task + has zero. +2. Admission implies progress, subject only to connect timeout and per-host cap. +3. A steal is slot-neutral. +4. Priority order for slot distribution beyond the first comes from DAEMON (queue order, + then FIFO by admission time, unless you'd rather take an explicit ordered list per tick + — ADR 0011 open item 4). + +If any of this can't be built as stated inside the stealer, say so before `sched/` exists — +this is the one thing in this doc that changes DAEMON's design rather than just its +plumbing. + +## C6. Probe pool sized outside the segment budget + +ADR 0011 §5: `download.probe` must not spend a segment slot, and `capture.offer`'s 750 ms +deadline can't wait on the segment budget being free. Confirming CORE's probe path already +runs its own small worker pool (proposed size 4) independent of `maxActiveSegments` — if +not, this is a request to make it so. + +--- + +Raise objections against ADR 0011 directly; this doc is downstream of it and changes if +the ADR does. 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..4c90819 --- /dev/null +++ b/docs/adr/0011-admission-control-and-the-segment-budget.md @@ -0,0 +1,182 @@ +# ADR 0011 — Admission control, the segment budget, and who counts what + +**Status:** proposed · **Date:** 2026-09-09 · **Lane:** DAEMON, needs CORE sign-off +**Companion request:** `daemon/docs/core-requests-m1.md` (the engine API this depends on) +**Blocks:** `daemon/src/sched/` — no scheduler code lands before this is accepted. + +## Context + +Two lanes are each building a global concurrency governor, neither brief mentions the +other, and they are counting different things. + +* **CORE** is adding `maxActiveSegments` (default 32) 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. + +### 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. + +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. + +### 3. CORE's fairness rule — what DAEMON is allowed to assume + +CORE owns this. It is recorded here because DAEMON's governor is built on top of it. + +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. +2. Beyond the first slot, remaining budget is distributed round-robin over running tasks in + the priority order DAEMON supplies, 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. **A steal is + slot-neutral** — the stealing worker already holds the slot it re-ranges. +4. A paused task holds no slots. +5. Therefore **admission implies progress**: a task DAEMON starts gets at least one + transferring segment, subject only to the connect timeout and the per-host cap. +6. **Invariant:** `budget().tasks_starved == 0` in steady state. DAEMON asserts this. If it + observes a non-zero value persisting past 2 s it logs a governor-invariant warning and + surfaces it in `velox ls --json`; 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** take the +whole budget away from the next, because rule 1 makes the next task's first slot outrank +the incumbent's second. A single download alone in the system does legitimately grow to 32 +segments, and gives slots back as tasks arrive — growth is opportunistic, the first slot is +guaranteed. + +### 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"). +* 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()`. 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 small pool +(proposed: 4) outside the segment budget. `capture.offer` never blocks on a probe under any +circumstances — it answers `ignore` first and probes after. + +### 6. Contract gap — `connection.maxActiveSegments` does not exist on the wire yet + +`Settings.schema.json` has `maxSegmentsPerDownload`, `bufferBytes`, +`maxConcurrentDownloads`; it has neither `maxActiveSegments` nor `maxTotalBufferBytes`. +PROTO accepted the B2a follow-up bundle (`contracts/proto-answers-m1.md`) but it has not +landed, and `SettingKey` is a closed enum against `additionalProperties: false` — a daemon +**cannot** add the key locally without failing conformance. + +Until PROTO lands it: DAEMON holds `maxActiveSegments` as a daemon-local value defaulted to +32, passes it to `set_max_active_segments()` at startup, and does not expose it through +`settings.get` / `settings.set`. Filed as request C4 in `daemon/docs/core-requests-m1.md` +(to PROTO, alongside CORE's existing ask). + +## 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 needs an occupancy read-out rather than an inference — the engine API requested in + `daemon/docs/core-requests-m1.md` §1 (`budget()`, `segments_active(TaskId)`, a coalesced + change callback). Without it, projecting `TaskSummary.segments` (which ADR 0010 pinned to + the *effective* count) is guesswork. +* CORE's fairness rule needs a test that DAEMON can point at: N tasks admitted, budget + smaller than N × their per-task caps, assert every task holds ≥ 1 segment. +* If CORE cannot honour min-1 for a reason not anticipated here, this ADR is wrong rather + than incomplete — say so before `sched/` exists, which is the entire point of settling it + now. + +## Open, for CORE to confirm or amend + +1. Min-1 before seconds (§3.1) — is it implementable inside the stealer without a + priority inversion at slot release? +2. Probe pool outside the segment budget, size 4 (§5). +3. Does `set_max_active_segments()` apply live, draining as slots free rather than killing + in-flight segments? +4. `Priority` shape: an integer, or DAEMON handing over an ordered task list per tick? +5. Coalescing rate for the budget-change callback — 4 Hz to match the event-batching rate? From 67b7b75336ab42136007bcc60388d017398c19ab Mon Sep 17 00:00:00 2001 From: sami Date: Wed, 9 Sep 2026 23:24:39 +0400 Subject: [PATCH 2/2] daemon: accept ADR 0011 with CORE's sign-off; land the engine API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CORE reviewed and accepted (core/docs/adr-0011-core-response.md, lane/core@7bf5cb5), with three amendments folded in: - yield (slot transfer at the next segment boundary) as the mechanism that satisfies min-1-before-seconds out of a full budget; steal stays slot-neutral as originally written. - "admission implies progress" is bounded-delay (min(next yield boundary, low_speed_secs) + connect_timeout), not immediate — widens the starvation-assertion window from 2s to ~low_speed_secs + connect_timeout (45s). - starved_tasks()/starved_since(TaskId) added to the accessor set; segments_active() and tasks_starved definitions pinned (a 'connecting' segment counts as held, not starved). All five open questions answered (min-1 buildable without inversion, probe pool size 4 outside the budget, drain-not-kill live-apply, ordered TaskId list for priority, 4Hz + starved-edge callback coalescing). Section 6 rewritten: connection.maxActiveSegments landed on the wire in PROTO's ADR 0012 while this was in flight, so the daemon-local stopgap is dropped. daemon/src/sched/ is unblocked. Both docs updated in the rebased vdm-daemon worktree against the frozen 1.0.0 contract. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig --- daemon/docs/core-requests-m1.md | 105 +++++----- ...dmission-control-and-the-segment-budget.md | 181 ++++++++++++------ 2 files changed, 170 insertions(+), 116 deletions(-) diff --git a/daemon/docs/core-requests-m1.md b/daemon/docs/core-requests-m1.md index 1d3ebf8..6a2e06f 100644 --- a/daemon/docs/core-requests-m1.md +++ b/daemon/docs/core-requests-m1.md @@ -1,78 +1,67 @@ # DAEMON → CORE — the engine API `sched/` needs before it can be written -Status: **open**. Companion to `docs/adr/0011-admission-control-and-the-segment-budget.md`, -which settles *who* enforces what. This doc is the concrete API ask that ADR depends on. -Also carries one contract gap (C4) that is PROTO's, not CORE's, filed here because it was -found while writing the ADR. +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 follows `contracts/README.md` rule 4 conventions even though this isn't a +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) = needs CORE's explicit sign-off before DAEMON builds on the assumption. +fairness) = needed CORE's explicit sign-off before DAEMON built on the assumption. That +sign-off is in. --- -## C1. Occupancy read-out, not inference +## C1. Occupancy read-out, not inference — **resolved** -DAEMON's governor (ADR 0011 §1) must never count segments to make an admission decision, -which means it needs to *read* engine occupancy rather than derive it from how many tasks -it thinks it started. Requesting: +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. -- `EngineBudget budget() const` — `{ total: uint32, active: uint32, tasks_starved: uint32 }` - where `tasks_starved` is the count of running tasks currently holding zero segments - (ADR 0011 §3.6 — DAEMON asserts this is 0 in steady state and logs if it isn't). -- `uint32_t segments_active(TaskId) const` — for `TaskSummary.segments` projection - (ADR 0010: the *effective* count) and for `velox ls --json`. -- A coalesced `on_budget_changed(callback)`, batched at the same ≤4 Hz as - `event.task.progress` (brief item 5) — DAEMON is not polling this on a tighter loop. +## C2. `set_max_active_segments(uint32_t)` live-apply — **resolved: drain, never kill** -## C2. `set_max_active_segments(uint32_t)` and its live-apply semantics +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. -Brief already has this as a settings value CORE enforces. DAEMON needs to know: does -lowering it live drain existing segments down to the new ceiling (segments finish, no new -ones start until under budget), or does it kill in-flight segments? DAEMON's assumption, -pending your answer, is drain-not-kill — a live cut to 8 must not abort 24 in-flight -segments and lose their partial ranges. ADR 0011 open item 3. +## C3. `set_host_segment_cap(host, uint32_t)` — **resolved, confirmed as proposed** -## C3. `set_host_segment_cap(host, uint32_t)` +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. -The per-host table is DAEMON state (SQLite `settings`/a host-cap table); CORE enforces it -in segment units. Requesting a push API so there is one source of truth for the table and -two enforcement points, per ADR 0011 §4. DAEMON derives its own per-host **task** cap from -the same table value — same number, different unit, pushed once. +## C4. Contract gap: `connection.maxActiveSegments` — **resolved by PROTO** -## C4. Contract gap: `connection.maxActiveSegments` isn't in `Settings.schema.json` +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. -Not CORE's item — flagging because ADR 0011 depends on it and `core/docs/buffer-sizing.md` -already asked for it. `SettingKey` is a closed enum (`additionalProperties: false`), so -DAEMON cannot expose this key through `settings.get`/`settings.set` until PROTO lands it. -Until then DAEMON holds the value (default 32) locally and passes it to -`set_max_active_segments()` at startup only — no wire exposure, so `Options → Connection` -can't show or set it yet. Same ask as CORE's B2a bundle; landing them together is fine. +## C5. Fairness rule sign-off — **resolved, with two amendments** -## C5. Fairness rule sign-off (ADR 0011 §3) +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: -Not new API — a behavioural contract. DAEMON's governor is built assuming: +- **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. -1. Min-1 before seconds: no running task gets a second slot while another running task - has zero. -2. Admission implies progress, subject only to connect timeout and per-host cap. -3. A steal is slot-neutral. -4. Priority order for slot distribution beyond the first comes from DAEMON (queue order, - then FIFO by admission time, unless you'd rather take an explicit ordered list per tick - — ADR 0011 open item 4). +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. -If any of this can't be built as stated inside the stealer, say so before `sched/` exists — -this is the one thing in this doc that changes DAEMON's design rather than just its -plumbing. +## C6. Probe pool sized outside the segment budget — **resolved, confirmed** -## C6. Probe pool sized outside the segment budget - -ADR 0011 §5: `download.probe` must not spend a segment slot, and `capture.offer`'s 750 ms -deadline can't wait on the segment budget being free. Confirming CORE's probe path already -runs its own small worker pool (proposed size 4) independent of `maxActiveSegments` — if -not, this is a request to make it so. - ---- - -Raise objections against ADR 0011 directly; this doc is downstream of it and changes if -the ADR does. +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 index 4c90819..0002e1e 100644 --- a/docs/adr/0011-admission-control-and-the-segment-budget.md +++ b/docs/adr/0011-admission-control-and-the-segment-budget.md @@ -1,17 +1,20 @@ # ADR 0011 — Admission control, the segment budget, and who counts what -**Status:** proposed · **Date:** 2026-09-09 · **Lane:** DAEMON, needs CORE sign-off +**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) -**Blocks:** `daemon/src/sched/` — no scheduler code lands before this is accepted. +**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 are each building a global concurrency governor, neither brief mentions the -other, and they are counting different things. +Two lanes were each building a global concurrency governor, neither brief mentioned the +other, and they were counting different things. -* **CORE** is adding `maxActiveSegments` (default 32) 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. +* **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. @@ -64,6 +67,9 @@ Corollaries, and these are the parts that actually prevent the two failure modes 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: @@ -76,69 +82,105 @@ effective_max_running_tasks = min(connection.maxConcurrentDownloads, 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. +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. It is recorded here because DAEMON's governor is built on top of it. +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. + 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, 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. **A steal is - slot-neutral** — the stealing worker already holds the slot it re-ranges. + 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. Therefore **admission implies progress**: a task DAEMON starts gets at least one - transferring segment, subject only to the connect timeout and the per-host cap. -6. **Invariant:** `budget().tasks_starved == 0` in steady state. DAEMON asserts this. If it - observes a non-zero value persisting past 2 s it logs a governor-invariant warning and - surfaces it in `velox ls --json`; it does **not** compensate by throttling admission. - Compensating is how the two governors would silently grow back into one. +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** take the -whole budget away from the next, because rule 1 makes the next task's first slot outrank -the incumbent's second. A single download alone in the system does legitimately grow to 32 -segments, and gives slots back as tasks arrive — growth is opportunistic, the first slot is -guaranteed. +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"). + 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()`. One source of truth, two enforcement points, different units. + `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 small pool -(proposed: 4) outside the segment budget. `capture.offer` never blocks on a probe under any -circumstances — it answers `ignore` first and probes after. +`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. Contract gap — `connection.maxActiveSegments` does not exist on the wire yet +### 6. `connection.maxActiveSegments` is now on the wire (PROTO ADR 0012) -`Settings.schema.json` has `maxSegmentsPerDownload`, `bufferBytes`, -`maxConcurrentDownloads`; it has neither `maxActiveSegments` nor `maxTotalBufferBytes`. -PROTO accepted the B2a follow-up bundle (`contracts/proto-answers-m1.md`) but it has not -landed, and `SettingKey` is a closed enum against `additionalProperties: false` — a daemon -**cannot** add the key locally without failing conformance. - -Until PROTO lands it: DAEMON holds `maxActiveSegments` as a daemon-local value defaulted to -32, passes it to `set_max_active_segments()` at startup, and does not expose it through -`settings.get` / `settings.set`. Filed as request C4 in `daemon/docs/core-requests-m1.md` -(to PROTO, alongside CORE's existing ask). +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 @@ -161,22 +203,45 @@ Rejected: one counter, one owner, read-only snapshots for everyone else. * `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 needs an occupancy read-out rather than an inference — the engine API requested in - `daemon/docs/core-requests-m1.md` §1 (`budget()`, `segments_active(TaskId)`, a coalesced - change callback). Without it, projecting `TaskSummary.segments` (which ADR 0010 pinned to - the *effective* count) is guesswork. -* CORE's fairness rule needs a test that DAEMON can point at: N tasks admitted, budget - smaller than N × their per-task caps, assert every task holds ≥ 1 segment. -* If CORE cannot honour min-1 for a reason not anticipated here, this ADR is wrong rather - than incomplete — say so before `sched/` exists, which is the entire point of settling it - now. +* 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). -## Open, for CORE to confirm or amend +## Engine API `sched/` is built against (CORE, `core/docs/adr-0011-core-response.md`) -1. Min-1 before seconds (§3.1) — is it implementable inside the stealer without a - priority inversion at slot release? -2. Probe pool outside the segment budget, size 4 (§5). -3. Does `set_max_active_segments()` apply live, draining as slots free rather than killing - in-flight segments? -4. `Priority` shape: an integer, or DAEMON handing over an ordered task list per tick? -5. Coalescing rate for the budget-change callback — 4 Hz to match the event-batching rate? +``` +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.