merge: ADR 0011 accepted — admission control and the segment budget
This commit is contained in:
@@ -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.
|
||||
@@ -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<const TaskId>); // 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<TaskId> starved_tasks() const;
|
||||
std::optional<SteadyTime> starved_since(TaskId) const;
|
||||
void on_budget_changed(std::function<void(EngineBudget)>); // 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.
|
||||
Reference in New Issue
Block a user