Skip to content

lanes: one fixed code for every wait and refusal (#2135) - #2196

Draft
ashhart wants to merge 1 commit into
mainfrom
lead/wait-reasons
Draft

ashhart wants to merge 1 commit into
mainfrom
lead/wait-reasons

Conversation

@ashhart

@ashhart ashhart commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Draft for #2135: one fixed code for every wait and refusal.

The matrix. Only DeepSeek-V4.1 mixes prompt and decode rows in one round window. Every other lane-host family with a prefill step interleaves separate prompt chunks between decode rounds, whole-prompt families hold a lane, and lone drivers and warm or prompt-only jobs never mix.

The codes. core/wait_reason.zig holds fixed Wait and Refusal codes. A queued job is labelled by the path holding it, and refusals are counted where they happen. /metrics adds requests_waiting_by_reason{reason=...} and request_refusals_total{reason=...} with every label at 0, and /v1/load adds waiting_by_reason and refusals.

This touches lane_host files for labels and counts only, with no scheduling change, and needs review by whoever owns that code. Host tests were run on an older main; this head hasn't been built since its rebase. Kept as a draft while we agree with @devopsfortunato, who has claimed #2135.

@ashhart

ashhart commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

Notes from review, so nobody spends time finding them again: waitReason calls admit.fits per queued job on every publish, inside the host's unlock, which adds work and locks on the admit path. That needs a different shape before this leaves draft. Also, kv_pool, memory_floor and prefill_concurrent are never set, restore_refused counts a request that then succeeds, and DeepSeek's memory floor needs its own code. This stays a draft until we've agreed the shape with @devopsfortunato.

Publish fixed wait and refusal reasons in the load ledger, JSON snapshot,
metrics and DeepSeek admission logs. Reuse engine_busy,
engine_unavailable and memory_pressure from the HTTP errors; closed and
stale submissions retain their original errors and share the unavailable
counter. Additional codes describe prefix, state-read, cache-budget and
backend conditions.

Keep HTTP responses and scheduling decisions unchanged. Move the existing
submission body to its own module to keep the host within the source
limit. Observe only priced request ledgers through their price callback;
a page-backed pool has no per-request price.

Check shared HTTP and diagnostic names, repeated memory refusals, queue
reason counts, preserved closed/stale errors and page-backed observation.

With a load producer attached, each host unlock scans queued requests and
recomputes wait reasons under the host/load locks before deduplication,
including admission callbacks where needed; elapsed overhead is unmeasured.
@devopsfortunato

Copy link
Copy Markdown
Contributor

Row-by-row check of this draft's counters against zig/src/server/wait_reason.zig, as asked on #2135. I read head 9932998e5, built and ran the new step, and compared every client-facing code with the counter labels.

Shared names hold. server/wait_reason.zig:19-21 derives the three capacity codes from core.wait_reason.waitCode, so the counter labels are the same strings a client reads:

HTTP code Counter reason(s) here Same string
engine_busy Wait.engine_busy, Refusal.engine_busy yes
engine_unavailable Wait.engine_unavailable, Refusal.engine_unavailable yes
memory_pressure Wait.memory_pressure, Refusal.memory_pressure yes
server_restarting — no counter needed: a draining server queues nothing
engine_loading, engine_reload_failed — no counter needed: the request never enters a lane queue

The 19 series (10 waits + 9 refusals) are a fixed set, so the labels stay bounded.

1. DeepSeek-V4.1's three planner reasons never reach the gauge. Wait.kv_pool, .memory_floor and .prefill_concurrent (core/wait_reason.zig:13-15) are produced by no path in waitReason() (core/lane_host_load.zig:219-229); they appear only in sched_host.zig's log lines (:129, :131, :132). A dsv41 host's verdict arrives through Rounds.vtable.admit (sched_host.zig:87-118), not Info.admit, and waitReason consults only the latter (lane_host_load.zig:227). So a request queued on KV-pool pages or the memory floor is counted as engine_busy while its log line says kv_pool — for the one family the issue says mixes prompt and decode rows. Having the verdict carry the reason (.wait: reasons.Wait) or storing the planner's last reason per job would make the log and the gauge agree; a small dsv41 test with a planner returning .wait for each of the three would pin it.

2. restore_refused and the two backend reasons are not refusals. lane_job.zig:71 notes .restore_refused when reuse_failed, but that request continues and prefills from zero: the client gets a normal reply. backend_prompt/backend_round are dropped requests (500/503), not refusals either. The help text at server/load_metrics.zig:36 says "Refused requests by fixed reason", so the family's name is narrower than its contents. Either rename it (request_failures_by_reason) or count the restore fallback apart (restore_fallbacks_total).

3. The new fields are undocumented. This adds waiting_by_reason and refusals to /v1/load and 19 series to /metrics, but no docs file changes. docs/api.md:634 is the /v1/load reference and :688 the Waits table; both should name the new keys and say the wait gauge sums to waiting.

4. Two simplifications.

  • refusalCode() (core/wait_reason.zig:39-47) is @tagName(r) behind a switch: all three arms return waitCode(...), which is @tagName, and else returns @tagName(r). return @tagName(r); is the same function, and the test at :121-128 already pins the shared names.
  • Waits.total() has no production caller. The invariant it exists for (sum is waiting, lane_host_load.zig:206 + :213, load_metrics.zig:32) would be worth an assert in requests(), where the lock is already held — or the method could go.

The draft as it stands (head 9932998e5, aarch64 Linux, no GPU): zig build test-load-metrics-host 3/3 steps, 25/25 tests pass (compile 18 s, run 57 ms); zig fmt --check clean on the ten changed Zig files. It is 107 commits behind main (54336bc5 vs 9f4e1ec7) and touches lane_host.zig/lane_host_load.zig, where #2202 and the docs sweep have since landed, so a rebase will conflict in those files.

On your note about waitReason under the host lock: confirmed. requests() is documented as called with the mutex held (lane_host_load.zig:193) and waitReason calls admit.fits for every queued job on every publish (:227), so the fix has to count on transition (or read a per-job reason) rather than re-derive it per publish.

@devopsfortunato

Copy link
Copy Markdown
Contributor

Here is the shape I would agree to, answering the four points from your review note of 5 Nov plus the four findings in my check above. It is a proposal, not a patch: tell me where you want it different and I will rework whichever part you prefer to carry.

1. Where a wait reason comes from (your point 1: admit.fits per queued job, inside the host's unlock).

The reason should be decided once, on the admission path, and stored on the job; the gauge then only reads integers.

  • api.Rounds.Verdict.wait carries the reason: .wait: reasons.Wait. The family's planner names it, so DeepSeek-V4.1's own planner answers .kv_pool, .memory_floor or .prefill_concurrent (it already knows which of the three it is waiting on - sched_host.zig:127-132 reads it off pool_waits/mem_waits today, and logWait is the only place that knowledge exists).
  • The host stores that reason on the job (job.wait_reason), and clears it when the job is admitted.
  • waitReason() in lane_host_load.zig:219-229 becomes a read of the stored reason, with the host's own reasons (stale, a kept state still being read, pressure) decided at the moment the job is queued or re-examined, not re-derived on every publish.
  • Info.admit.fits is then called once per admission attempt, where the host already calls the planner, instead of once per queued job on every publish. The publish path keeps the lock it already holds and only copies counters.

If you would rather not put the reason in the verdict, the alternative is the host asking the planner once per round for the head it did not admit; the verdict is cheaper and cannot race with the round that follows.

2. The codes (your points 2 and 4).

  • Internal reasons stay in core/wait_reason.zig; the client-visible codes stay in server/wait_reason.zig. The three shared names (engine_busy, engine_unavailable, memory_pressure) are the same strings in both, as they are now.
  • Your point 2 is a consequence of point 1: kv_pool, memory_floor and prefill_concurrent are never set because no path produces them - waitReason() only consults Info.admit, which DeepSeek-V4.1 does not set (serve_lanes.zig:136 and serve_engine.zig:132 set no admit). With the verdict carrying the reason, the planner sets them.
  • Your point 4, "DeepSeek's memory floor needs its own code": I read that as the memory-floor wait must not be counted under memory_pressure, because that is the critical-pressure refusal and the two are different events. Proposal: memory_floor stays an internal-only reason, never a client code, and the planner returns it. If you meant a client-visible code instead, say so and I will name it and add the row to the Waits table - that is a one-line change once the shape is fixed.
  • The other host-side reasons (prefix_cut, state_read, ledger_bytes, pool_room) are real and stay.

3. The counters (your point 3: restore_refused counts a request that then succeeds).

  • request_refusals_total should count exactly what a client can see as a refusal: the three 503 codes and the named 400s. So Refusal keeps engine_busy, engine_unavailable, memory_pressure and invalid_request (the 400's invalid_request_error).
  • restore_refused leaves that counter entirely: the request is not refused, it is served after prefilling from zero. Proposal: a separate counter restore_fallbacks_total - one new name, a counter, and the restore fallback is visible without pretending it was a refusal.
  • backend_prompt, backend_round, lane_memory_alone and prompt_too_large are drops (500/503) or family-level refusals with their own bodies, not client refusals. Proposal: a second family request_failures_total with those reasons. Keeping one mixed family would need the name request_failures_by_reason, which then does not match its help text ("Refused requests by fixed reason").
  • refusalCode() can then be @tagName(r) (the switch is a no-op today) and Waits.total() earns its place as the assert for the documented invariant.

4. The invariant and the docs.

  • sum(waiting_by_reason) == waiting holds by construction - one reason per queued job - and is asserted where the ledger is read, so a drift is a test failure rather than a number nobody checks.
  • docs/api.md's "Scheduler load" section (line 634) gains waiting_by_reason and refusals, and says which names are client codes and which are internal reasons; the Waits table (line 688) gains a row only if point 4 turns into a client-visible code.

What I can do, whichever way you want it:

  • A. You take this shape into lanes: one fixed code for every wait and refusal (#2135) #2196; I review the result and run whatever receipts you want on the DeepSeek side.
  • B. I implement it on my own branch against main and open a pull request, and you rework or close the draft. The pieces are: the verdict carrying a reason, the host storing it per job, the ledger reading stored reasons, the counter split above, the docs, and a DeepSeek host test whose planner returns each of the three waits so the reasons are pinned by a test rather than by a log line.

Either way the DeepSeek reasons need a test before they can be claimed: a planner test double that returns .wait for each of the three, and a ledger read that shows kv_pool, memory_floor and prefill_concurrent instead of engine_busy.

One thing I can answer now, so it does not wait for the shape: the three shared names in the draft are already the strings the client sees - server/wait_reason.zig:19-21 derives them from core.wait_reason.waitCode - so a client sees one set of codes whichever way the counters are split.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants