fix(server): dispatch a lone idle request without the coalesce window

[YeonwooSung]

Summary

An idle model worker held every request for idle_coalesce_ms (default 3ms) and then polled with asyncio.sleep(0.001). Sequential clients paid that delay on every request. The window exists so a burst that reaches an idle worker is not split into one forward per request.

A request that is still alone is now dispatched immediately. One later item that arrives inside the idle window is treated as the tail of a staggered burst and still coalesces, so the first request of that burst can run alone and the rest fuse behind it. Several items already pending when a batch is selected still form one batch. idle_coalesce_ms=0 still dispatches every idle arrival immediately. The idle loop waits on an event that submit() sets instead of sleeping 1ms.

The default window stays 3ms. Cost-ordered batching is unchanged.

Compatibility

Scheduling latency only. A lone request on an idle worker is no longer delayed by the coalesce window. A staggered burst may use two forwards (the first alone, the rest together) instead of one fused forward.

Tests

pytest packages/sie_server/tests/core/test_worker_core.py packages/sie_server/tests/core/test_worker_drain.py — 39 passed.

A unit test sets idle_coalesce_ms high and checks a lone request finishes well under that window. It does not assert a hard 1ms, which is flaky under the test scheduler.

Refs #373

Summary by CodeRabbit

• Bug Fixes
  • Idle requests are dispatched promptly instead of waiting for the coalescing window. A later request waits only while another request for the same model is pending; completed requests and requests for other models do not delay dispatch.
  • Requests already queued together continue to be processed as a batch. Setting the coalescing window to zero dispatches each idle arrival immediately.
  • The coalescing wait is limited by the configured maximum batch wait, so an idle batch cannot remain open indefinitely.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2082c8c8-a590-456d-82b8-2bf9f337c8eb

📥 Commits

Reviewing files that changed from the base of the PR and between de5cb67 and 8a2d0cc.

📒 Files selected for processing (4)

• packages/sie_server/src/sie_server/core/batcher.py
• packages/sie_server/src/sie_server/core/worker/model_worker.py
• packages/sie_server/tests/core/test_worker_core.py
• packages/sie_server/tests/core/test_worker_drain.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The worker dispatches a lone idle request immediately. When multiple requests remain pending on the same batcher, it applies the configured idle coalescing window. An event wakes the batch-processing loop when work arrives or the worker stops.

Changes: Idle Batch Dispatch

Layer / File(s)	Summary
Idle window contract and pending counts packages/sie_server/src/sie_server/config/engine.py, packages/sie_server/src/sie_server/core/worker/types.py, packages/sie_server/src/sie_server/core/batcher.py	Configuration descriptions define idle_coalesce_ms as a tail window for requests pending on the same batcher. pending_count reports pending items, and the new pending_request_count property reports distinct pending requests.
Worker batch selection packages/sie_server/src/sie_server/core/worker/model_worker.py	The worker signals an event on submission and shutdown, and waits on the event instead of polling when no batch is pending. While idle, it dispatches one pending request immediately and applies the capped window when multiple requests remain pending on the selected batcher.
Batching behavior tests packages/sie_server/tests/core/test_worker_core.py, packages/sie_server/tests/core/test_worker_drain.py	Tests cover lone-request dispatch, tail fusion, already-pending bursts, zero-window dispatch, sequential requests, and arrivals on different LoRAs. Drain tests use partner requests to hold the coalescing window and check pending item counts.

Sequence Diagram(s)

sequenceDiagram
  participant Submitter
  participant ModelWorker
  participant Batcher
  Submitter->>ModelWorker: Submit items
  ModelWorker->>Batcher: Add items to selected batcher
  ModelWorker->>ModelWorker: Signal work-availability event
  ModelWorker->>Batcher: Check pending request count
  Batcher-->>ModelWorker: Return pending request count
  ModelWorker->>Batcher: Select batch immediately or after tail window

Loading

Suggested reviewers: krisztian-gajdar

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 8a2d0

No actionable merge-blocking risk is identified; the change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8a2d0

The inspected changes adjust request timing without adding access or privileges. Queue limits and shutdown cleanup remain intact in the reviewed paths. No introduced security concern was identified, but broader security and deployment coverage is incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly affected shared resource is inference scheduling within an existing loaded-model worker. Request timing can influence batch formation, but the inspected change does not expand tenant, credential, data-store, or environment authority. Deployment-level exposure was not established by the available evidence.

Trust Boundaries and Controls

• observed — Inspected upstream paths continue to obtain workers through registry-controlled lifecycle checks and prepare and validate request payloads before worker submission. The public-API routing signals describe existing Python exposure; this PR adds no network route or bypass of those inspected controls.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: idle workers dispatch a lone request without waiting for the coalesce window.
Docstring Coverage		Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 6 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[krisztian-gajdar]

Thanks, the event-based wakeup looks right to me, and the tests drive the real worker and batcher without patching the clock. A few things before merge:

1. Multi-item requests. selected.pending_count == 1 (model_worker.py:1147) counts pending items, not requests, because submit_many adds one entry per item. A lone request with two or more items (batch encode, a multi-document rerank, multi-item extract) still waits the full window. Could you decide "alone" per request and add a multi-item test?
2. Tail check. _follows_recent_arrival compares against the previous submit even after that request has finished, so a sequential client sending faster than the window still waits. The timestamps are also worker-wide while pending_count is per LoRA batcher. Please either adjust this or document it in the docstring.
3. Throughput. The staggered-burst test now expects [1, 3] instead of one fused batch. #373 asks that throughput under concurrent load stays within noise, so a short before and after on one GPU (sequential queue time, plus throughput at 8 and 32 in flight) would let us accept that trade.
4. Small. The "instead of polling every 1 ms" comment at model_worker.py:288-289 narrates the change, and the drain tests seed _last_arrival_at directly. Parking them with a two-item submit would avoid reaching into private state.

[YeonwooSung]

Addressed on 8dfac7b.

1. Multi-item requests. Alone is one request, not one queue entry. submit_many still adds one pending item per input, and those items share one metadata object. BatchFormer.pending_request_count counts distinct metadata. One pending request is dispatched immediately, however many items it carries. test_multi_item_request_does_not_wait_the_idle_window does one submit of two items with idle_coalesce_ms=200 and finishes well under that window, as one forward of 2. Several requests already pending still form one batch (test_already_queued_burst_fuses expects [4]).

2. Tail check. The worker-wide arrival timestamps are gone. A follow-up is a tail only while another request on that same batcher is still pending (it has arrived and has not been dispatched). A finished request does not hold the next one: test_sequential_request_does_not_wait_the_idle_window awaits the first, submits the next immediately, and the second finishes well under a 200ms window. A different LoRA's arrival does not hold this batcher: test_other_lora_arrival_does_not_hold_this_batcher.

3. Throughput. nvidia-smi is not installed here (command not found), so this machine has no GPU I can measure. I am not reporting a before/after and I am not claiming a GPU result. The unit test that documents the two-forward split is unchanged in what it asserts: test_staggered_burst_fuses_with_idle_window still expects [1, 3].

4. Small. Removed the "instead of polling every 1 ms" comment. The event note only records that stop() sets it so an idle loop unblocks. Drain tests no longer write a private arrival timestamp. A two-item submit is a single request and would now dispatch immediately, so it cannot keep work parked until stop(). The drain helper submits a second request on the same batcher through the public submit() API, which is what the idle window holds.

Default idle_coalesce_ms is still 3. Batcher cost ordering is unchanged.

pytest packages/sie_server/tests/core/test_worker_core.py packages/sie_server/tests/core/test_worker_drain.py — 42 passed.

[krisztian-gajdar]

Thanks, counting distinct requests and dropping the arrival timestamps fix both points, and the new tests cover them.

Understood on the GPU. The throughput question still has to be answered before this can merge, because with the new rule a burst that is not already queued costs at least two forwards. So this will stay open until a before and after measurement on one GPU is available (sequential queue time, plus throughput at 8 and 32 in flight).

One test issue in the meantime: test_staggered_burst_shreds_without_idle_window (test_worker_core.py:713) still submits with a 5 ms stagger. Under the new rule that stagger also splits with the idle window on, so the test no longer shows what the window adds, and its docstring's "the same stagger as test_staggered_burst_fuses_with_idle_window" no longer holds since that test now uses the queued-tail pattern. Reusing the fusing test's pattern with idle_coalesce_ms=0 would make it a real contrast.

Optional: test_already_queued_burst_fuses patches the private _ensure_process_loop_started, and pending_request_count could be checked only when the worker was idle and idle_coalesce_ms > 0.

[YeonwooSung]

The throughput hold stands. I ran command -v nvidia-smi, which printed nvidia-smi: not found, and invoking nvidia-smi exited 127 with command not found. This machine has no NVIDIA GPU, so there is no sequential queue time and no throughput at 8 or 32 in flight. I am not reporting a before/after and I am not claiming a GPU result. I did not add a benchmark script. A burst that is not already queued still costs at least two forwards, and this stays open until that measurement exists on one GPU.

test_staggered_burst_shreds_without_idle_window no longer uses a 5 ms stagger. It now follows the queued-tail shape of test_staggered_burst_fuses_with_idle_window, with the same coalesce settings and idle_coalesce_ms=0. The head runs alone. Two requests are queued before the loop selects, and one more arrives 10 ms later. With the window on, that tail is held and the later arrival joins ([1, 3]). With the window off, the already-queued pair is dispatched immediately (pending_count is 0 after 10 ms) and the later arrival is its own forward ([1, 2, 1]). The fuse test is unchanged.

test_already_queued_burst_fuses still patches _ensure_process_loop_started. ModelWorker.start() does not start the batch loop; the loop starts lazily on the first submit, and that private helper is the only method that creates the task. There is no public way to queue several requests and then start the loop, so the patch stays.

pending_request_count was read on every selection, including the busy path and idle_coalesce_ms=0. It is now read only when the worker was idle and idle_coalesce_ms > 0; the and short-circuits otherwise. Dispatch behavior is otherwise the same. The default idle_coalesce_ms is still 3, and batcher cost ordering is unchanged.

pytest packages/sie_server/tests/core/test_worker_core.py packages/sie_server/tests/core/test_worker_drain.py — 42 passed. Commit de5cb67.