Skip to content

feat(broker): opt-in round-robin turns (max turn, rotation, preemption) and gateway reply timeout - #276

Open
chapi456 wants to merge 2 commits into
T-Crypt:mainfrom
chapi456:feat/round-robin-turns
Open

chapi456 wants to merge 2 commits into
T-Crypt:mainfrom
chapi456:feat/round-robin-turns

Conversation

@chapi456

Copy link
Copy Markdown

Refs #275

What

Two opt-in additions for a card shared by engines that each want it for a while (for example an LLM server and an image generator). Every setting is off by default; without configuration Berth decides exactly as before.

1. [turns]: round-robin turns (broker)

[turns]
max_turn    = "15m"            # longest a holder keeps the card against a lower priority
cycle       = ["llm", "image"] # who is served next among equal priorities
preempt     = true             # a higher priority may cut a min_turn short...
preempt_gap = "45s"            # ...at most once per this long (default 45s)

[turns.max_turn_by]            # per engine (name or --group), over max_turn
image = "30m"
  • Maximum turn. Today a higher-priority holder keeps a lower-priority waiter out for as long as it runs. With max_turn, a holder that has had the card that long stops outranking a waiting lower priority, which drains it at the next boundary. Holds and pinned leases are never affected.
  • Rotation. Waiters of one priority are served by arrival. With cycle, the engine after the last one granted goes first (llm, image, llm, image, ...). Unlisted engines go after the listed ones, by arrival.
  • Preemption. min_turn keeps every waiter out, however urgent. With preempt = true, a waiter of a strictly higher priority may drain a holder inside its min_turn, at most once per preempt_gap. Each such cut makes the engine it empties reload from cold (about two minutes of idle GPU in my case), so the gap bounds the cost. Inside the gap the waiter is told when the next preemption is allowed. Equal or lower priorities still wait out the turn.

None of it cuts a request off: a drain still starts at the slot or job boundary and the driver must still show the memory free before the next grant.

2. [gateway] reply_timeout

An engine that takes a chat request and sends no response headers within reply_timeout gets the request once more (started again if it went away). A second silence answers 504 with code no_reply (an SSE event for streaming requests) instead of leaving the client hanging. This is the "pending request never gets a response" half of #275 point 1; the 409 on /v1/unload and the latch themselves are not changed here.

Diagrams

stateDiagram-v2
    [*] --> Idle
    Idle --> Turn: grant
    state Turn {
        [*] --> Protected
        Protected --> Open: min_turn ends
        Open --> Expired: max_turn ends
    }
    note right of Turn
        Protected: equal or lower priority waits;
        higher priority preempts if allowed,
        once per preempt_gap
        Open: equal priority drains the holder,
        lower priority waits
        Expired: any waiting lower priority
        may drain the holder
    end note
    Turn --> Draining: a waiter may take the card
    Draining --> Idle: the driver shows the memory free
    Idle --> Turn: next waiter (cycle picks among equal priorities)
Loading
sequenceDiagram
    participant A as LLM (interactive)
    participant B as image generator (batch)
    participant C as person (foreground)
    A->>A: granted, turn starts
    B-->>A: waits (lower priority, held)
    Note over A: max_turn ends
    A->>B: drained at the next boundary
    B->>B: granted, min_turn 45 s protects it
    C-->>B: asks for the card at 20 s
    Note over B: preempt = true: the turn is cut short
    B->>C: drained
    C->>C: granted
    Note over C: another foreground request within 45 s of this preemption waits for the gap
Loading

Where

  • internal/broker/turns.go (policy, turnExpired, preempts, cycleRank), small hooks in evaluate, schedule, grant, sortQueue
  • internal/config ([turns], [gateway] reply_timeout, validation), internal/cli (wiring, a line in berth config show when something is on)
  • internal/gateway/gateway.go (proxyTo, one replay, 504)
  • docs/leasing.md (new section with the diagrams), docs/gateway.md, mkdocs.yml (Mermaid fence), CHANGELOG.md

Tests

internal/broker/turns_test.go (no policy keeps a higher-priority holder, max turn, per-engine max turn, holds exempt, cycle vs arrival, preemption off / on / equal priority / once per gap), internal/config (defaults off, parsing, validation), internal/gateway/replytimeout_test.go (silent engine reported and replayed, plain proxy unchanged, 504 JSON and SSE).

Run on Windows with Go 1.27.1: go build ./... && gofmt -l . && go vet ./... && go test ./... is green when the machine is not loaded; GOOS=windows build and vet are green. Three APU tests, TestUpdateCheckReportsNewer and TestInsightsComputedOncePerInterval failed once each under heavy load during a full parallel run and pass alone and on re-run (timing-based, none touch this change). golangci-lint v2.14.0 run on Windows reports 6 findings, all in files this PR does not touch (gpu/pdh_windows.go, config/credential.go, host/host.go, pagecache/pagecache.go); CI's Linux run is the reference.

Not done

🤖 Generated with Claude Code

chapi456 and others added 2 commits October 31, 2026 13:39
Add a [turns] table, off by default:
- max_turn / max_turn_by: a holder past its maximum turn stops outranking a
  waiting lower priority (holds and pinned leases excepted).
- cycle: waiters of one priority are served in a configured order, starting
  after the last engine granted.
- preempt / preempt_gap: a strictly higher priority may drain a holder inside
  its min_turn, at most once per gap (default 45s).

Refs T-Crypt#275

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…then 504

With [gateway] reply_timeout set, an engine that takes a chat request and
sends no response headers in that time gets it once more (started again if it
went away); a second silence answers 504 with the code no_reply instead of
leaving the client hanging. Off by default. The changelog covers both this
and the turns commit.

Refs T-Crypt#275

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@T-Crypt

T-Crypt commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR, and for writing up #275 so carefully. The numbers in it made the bug easy to trace. The code is clean and the tests are good. On my machine this branch passes build, gofmt, vet, the Windows build and vet, go test ./... and golangci-lint.

I'm not going to merge it as it stands. I think the bug you hit has a smaller fix that would help you more, so here is my reasoning.

What I think happened in point 1. I worked this out from the code, not from a repro. A resume runs on its own goroutine (runner.go:351). resume clears the parked flag (park.go:226) before it calls the engine's load (park.go:233). A drain that arrives in that window sees an engine that is not parked and calls park (runner.go:871). So /v1/unload reaches the engine while its /v1/load is still running. The engine answers 409, the park counts as failed, and the runner stops the engine. That matches the sequence in your report. min_turn only narrows it, because the drain can still land after the turn ends and before the load returns.

The fix is to make a drain wait for an in-flight resume before it parks. That is a few lines in internal/runner, plus a test that drains during a slow load and expects a single unload after the load returns.

Why reply_timeout doesn't fit. It answers the hang, but not the reason the request hung. It also relies on ResponseHeaderTimeout, and llama-server sends no headers for a non-streaming completion until the whole answer is ready. A long answer would be cut off, run a second time, then fail with 504. A safe value means knowing your longest generation in advance.

Why the turns policy doesn't fit. Each part changes how priority works for everyone who turns it on, and #275 asked for none of them. Preemption cuts min_turn short, which means more switches and more cold ComfyUI reloads. That is the cost you measured in point 2, and it would also widen the window for point 1. Rotation differs from arrival order only when two or more engines wait at once. Max turn reverses a priority the user set. Before any of these goes into the config, I'd want a real case from someone who needs it.

If you want to take the resume fix, it's yours, and I'll review it as soon as it's up. Please open it against next, which is where new work lands now. If you'd rather not, tell me and I'll do it and credit you in the commit. If you still want max turn or preemption after that fix, open an idea issue with the case that needs it and we can work it out there.

Points 2 and 3 stay open on #275.

This branch has not been deployed

No deployments
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