core: sign off on ADR 0011 with three amendments
Accept the decision (each ceiling enforced once by its unit owner; DAEMON
counts tasks, CORE counts segments; the single min() clamp; per-host caps
split by unit). daemon/src/sched/ is unblocked.
Answers to the five open questions:
1. Min-1 is implementable in the allocator without inversion: guarantee
pass (zero-slot tasks, priority order) before growth pass; a released
slot always re-enters allocation from the top, never handed back
locally.
2. Probe pool size 4, outside the segment budget — confirmed.
3. set_max_active_segments is drain-not-kill; in-flight segments run to
their boundary.
4. Priority = an ordered TaskId list pushed on change, not an integer,
not per tick — tie-breaking is DAEMON policy.
5. on_budget_changed coalesced at 4 Hz, immediate on the tasks_starved
zero-crossing.
Amendments: (A1) add "yield" to §3.3 as the non-neutral slot-transfer op
that satisfies min-1 when the budget is full — "steal" stays slot-neutral;
(A2) "admission implies progress" is bounded-delay not immediate — bounded
by an incumbent's next segment boundary, capped by the stall timeout, so
§3.6's 2 s assertion window is too tight; (A3) add starved_tasks() /
starved_since() and pin the segments_active / tasks_starved definitions
(a connecting segment counts as a held slot and is not starvation).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS
This commit is contained in:
Claude Sonnet 5
@@ -0,0 +1,177 @@
|
||||
# CORE response to ADR 0011 (admission control & the segment budget)
|
||||
|
||||
**Verdict: accept the decision, with three amendments and one caveat on §3.5.** None of
|
||||
the amendments change DAEMON's task-unit model — `daemon/src/sched/` is unblocked. The
|
||||
caveat tightens what "admission implies progress" can promise.
|
||||
|
||||
Signing off on §1 (each ceiling enforced once, by unit owner), §2 (the single clamp),
|
||||
§4 (per-host caps split by unit from one table), §6 (contract gap / daemon-local stopgap),
|
||||
and the three rejected alternatives — the shared-semaphore rejection especially.
|
||||
|
||||
---
|
||||
|
||||
## Answers to the five open questions
|
||||
|
||||
### Q1 — Min-1 before seconds: implementable in the stealer without inversion? **Yes.**
|
||||
|
||||
Slot allocation is a pure function of `(running tasks, their held-slot counts, their
|
||||
effective per-task caps, DAEMON's priority order, total budget)`. It runs to completion on
|
||||
every budget-changing edge (segment complete / start / pause / fail, task admit / pause /
|
||||
resume, a steal, a cap change), in two ordered passes:
|
||||
|
||||
1. **Guarantee pass.** Walk tasks holding **zero** slots *in DAEMON's priority order*; give
|
||||
each one slot while the pool is non-empty.
|
||||
2. **Growth pass.** While the pool is non-empty, round-robin over running tasks in priority
|
||||
order, granting one slot to any task below its effective cap
|
||||
(`min(spec.segments ?? maxSegmentsPerDownload, host_cap, resumable ? ∞ : 1)`), until the
|
||||
pool empties or nobody wants more.
|
||||
|
||||
The inversion DAEMON is worried about at slot release does not occur, because **a released
|
||||
slot always goes to the pool and the allocator re-runs from pass 1** — it is never handed
|
||||
back locally to the releasing task. If task A's segment finishes and both A and a
|
||||
lower-priority C now hold zero, the guarantee pass processes the zero-slot set *in priority
|
||||
order*, so A gets it back, not C. A task that drops to zero re-enters the guarantee queue
|
||||
at its priority position, not the back.
|
||||
|
||||
Liveness of min-1 depends on DAEMON honouring §2's clamp — never running more tasks than
|
||||
`maxActiveSegments`. If it admits 3 running tasks against a budget of 2, one starves by
|
||||
construction and no fairness rule fixes it. §2 already says this; calling it out because
|
||||
it is load-bearing for Q1.
|
||||
|
||||
### Q2 — Probe pool outside the segment budget, size 4: **confirmed.**
|
||||
|
||||
CORE's probe path is a dedicated pool, independent of `maxActiveSegments`. A probe is a
|
||||
`Request` with `method = HEAD` (or `range = {0,0}`) whose `on_head` returns `abort`; it
|
||||
never allocates a transfer slot. Default pool size 4, exposed as
|
||||
`set_probe_pool_size(uint32_t)`. Probe cancellation is immediate (a timed-out probe frees
|
||||
its pool slot at once), so `capture.offer` answering `ignore`-first-probe-after works.
|
||||
Probes still open real sockets — DAEMON should still bound probe *submission* on its side;
|
||||
CORE bounds *concurrency*, not queue depth.
|
||||
|
||||
### Q3 — `set_max_active_segments()` live-apply: **drain, never kill.**
|
||||
|
||||
- **Raise:** new slots available immediately; allocator runs; starved then growing tasks
|
||||
take them.
|
||||
- **Lower:** in-flight segments run to their next boundary. No new segment starts while
|
||||
`active > new_ceiling`. Completed segments' slots are withheld until
|
||||
`active ≤ new_ceiling`. Nothing is aborted, no partial range is lost.
|
||||
- **Edge:** if `new_ceiling < running_task_count`, CORE honours min-1 for the top
|
||||
`new_ceiling` tasks in priority order; the remainder are held at zero slots and reported
|
||||
in `tasks_starved`. CORE does **not** auto-pause them — that's policy. After a live
|
||||
lower, DAEMON must reconcile its running set against the new clamp (pause the
|
||||
lowest-priority excess).
|
||||
|
||||
### Q4 — Priority shape: **an ordered list, pushed on change — not an integer, not per tick.**
|
||||
|
||||
`set_task_order(std::span<const TaskId>)`, called by DAEMON whenever the order changes
|
||||
(admit, remove, reorder, priority edit, queue switch). CORE caches it and the allocator
|
||||
walks it. An integer priority would force CORE to implement tie-breaking (FIFO by
|
||||
admission time, queue precedence) — that's DAEMON policy and DAEMON already has the total
|
||||
order. Not per tick: an unchanged order isn't re-sent. A running task absent from the list
|
||||
sorts last (shouldn't happen; defensive).
|
||||
|
||||
### Q5 — Budget-change callback coalescing: **4 Hz for counts, immediate on the edge.**
|
||||
|
||||
`on_budget_changed` coalesced at ≤4 Hz to match `event.task.progress` — DAEMON isn't
|
||||
making sub-250 ms admission decisions. **Exception:** fire immediately when
|
||||
`tasks_starved` crosses 0→non-zero or non-zero→0, so DAEMON's invariant check and any UI
|
||||
reaction see that transition without up to 250 ms of lag.
|
||||
|
||||
---
|
||||
|
||||
## Amendments requested to the ADR
|
||||
|
||||
### A1 — §3.3 wording: distinguish *steal* from *yield*
|
||||
|
||||
"A steal is slot-neutral" is true for the steal the ADR means (a worker that finished its
|
||||
range takes the tail of the largest remaining range — the same worker, the same slot).
|
||||
Min-1 also needs a second, non-neutral operation:
|
||||
|
||||
- **Yield** — the allocator marks an over-quota task to release one slot *at its next
|
||||
segment boundary*. When that segment completes the slot goes to the pool → guarantee
|
||||
pass → starved task. It is a slot transfer, not slot-neutral, and it is bounded by the
|
||||
yielding segment's remaining bytes (never a mid-segment kill).
|
||||
|
||||
Please add "yield" to §3.3 as the mechanism that satisfies §3.1 when the budget is full.
|
||||
"Steal" stays exactly as written.
|
||||
|
||||
### A2 — §3.5 / §3.6: "admission implies progress" is bounded-delay, not immediate
|
||||
|
||||
When the budget is full of healthy incumbents, a newly admitted task's first slot
|
||||
materialises only when some incumbent segment reaches a boundary (yield) — bounded by that
|
||||
segment's remaining bytes, hard-capped by the stall timeout (`low_speed_secs`, default
|
||||
30 s, after which a stalled segment fails and frees its slot). So the true bound is
|
||||
|
||||
```
|
||||
time_to_first_slot ≤ min(incumbent's next boundary, low_speed_secs) + connect_timeout
|
||||
```
|
||||
|
||||
not "connect timeout + per-host cap" alone. Two consequences:
|
||||
|
||||
- §3.6's **2 s** assertion window is too tight — a legitimately full budget with a slow
|
||||
incumbent tail can hold a new task at zero for longer than 2 s with nothing wrong.
|
||||
Recommend the warning threshold be `low_speed_secs + connect_timeout_ms` (~45 s), or
|
||||
configurable.
|
||||
- CORE will expose `starved_since` (a monotonic timestamp) per starved task via the
|
||||
diagnostics call below, so DAEMON can tell "briefly waiting for a boundary" from
|
||||
"wedged" without guessing.
|
||||
|
||||
Optional future tightening (not M1): a **preemptive split** — truncate an incumbent's
|
||||
largest remaining range ahead of its current offset and hand the freed tail to the starved
|
||||
task as a new segment. Zero bytes lost, first slot within one round-trip. CORE will add
|
||||
this if the yield delay proves painful in the soak test; it doesn't change the API.
|
||||
|
||||
### A3 — C1 API: add a starved-set accessor and pin two definitions
|
||||
|
||||
```
|
||||
struct EngineBudget { uint32_t total; uint32_t active; uint32_t tasks_starved; };
|
||||
EngineBudget budget() const;
|
||||
uint32_t segments_active(TaskId) const; // slots held, any state
|
||||
std::vector<TaskId> starved_tasks() const; // diagnostics / velox ls --json
|
||||
std::optional<SteadyTime> starved_since(TaskId) const; // per A2
|
||||
void on_budget_changed(std::function<void(EngineBudget)>);
|
||||
```
|
||||
|
||||
Definitions, so the projection to `TaskSummary.segments` (ADR 0010: effective count) is
|
||||
unambiguous:
|
||||
|
||||
- **`segments_active(id)`** = slots the task holds, counting a segment in `connecting`
|
||||
(0 bytes yet) as well as `downloading`. This is what the user sees as "using N
|
||||
connections."
|
||||
- **`tasks_starved`** counts running tasks with `segments_active == 0`. A task with a
|
||||
`connecting` segment is **not** starved — it is progressing.
|
||||
|
||||
---
|
||||
|
||||
## C2–C6 confirmations
|
||||
|
||||
- **C2:** drain-not-kill, per Q3. Confirmed as DAEMON assumed.
|
||||
- **C3:** `set_host_segment_cap(std::string host, uint32_t)` — confirmed. CORE keeps the
|
||||
`host → cap` map, applies it in the per-task effective cap and in the stealer (no Nth
|
||||
connection to a host capped at N-1). DAEMON owns the table; CORE derives a task's host
|
||||
from its URL + mirror set.
|
||||
- **C4:** agreed it's PROTO's, same bundle as CORE's B2a / `buffer-sizing.md` asks
|
||||
(`connection.maxActiveSegments`, `connection.maxTotalBufferBytes`). DAEMON's local-value
|
||||
stopgap is fine.
|
||||
- **C5:** signed off — see Q1 plus amendments A1/A2. Min-1 is buildable; "implies
|
||||
progress" is bounded-delay; steal stays slot-neutral, yield is the transfer op.
|
||||
- **C6:** confirmed — see Q2.
|
||||
|
||||
---
|
||||
|
||||
## New API surface CORE will expose for `sched/` (summary)
|
||||
|
||||
```
|
||||
void set_max_active_segments(uint32_t); // drain-not-kill
|
||||
void set_host_segment_cap(std::string host, uint32_t);
|
||||
void set_task_order(std::span<const TaskId>); // pushed on change
|
||||
void set_probe_pool_size(uint32_t); // default 4
|
||||
EngineBudget budget() const;
|
||||
uint32_t segments_active(TaskId) const;
|
||||
std::vector<TaskId> starved_tasks() const;
|
||||
std::optional<SteadyTime> starved_since(TaskId) const;
|
||||
void on_budget_changed(std::function<void(EngineBudget)>); // 4 Hz + starved edge
|
||||
```
|
||||
|
||||
All of it lands with stage 6 (segmenter/stealer) / stage 8 (download_task). None of it is
|
||||
on the M1 critical path ahead of where DAEMON needs it; flag if the ordering is wrong.
|
||||
Reference in New Issue
Block a user