From ecedfac903b0329a89c6b2d1188401e217a78c2b Mon Sep 17 00:00:00 2001 From: sami Date: Wed, 9 Sep 2026 23:18:07 +0400 Subject: [PATCH] =?UTF-8?q?daemon:=20propose=20ADR=200011=20=E2=80=94=20ad?= =?UTF-8?q?mission=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?