fix: reserve completion-queue slot at enqueue to restore preserve_ordering

[mc-nv]

Problem

preserve_ordering in the dynamic batcher does not preserve ordering. Responses
are returned in completion order, not in the order requests reached the
scheduler.

model_config.proto is explicit that the guarantee is scheduler-wide:

Should the dynamic batcher preserve the ordering of responses to match the
order of requests received by the scheduler. [...] If true, the responses will
be returned in the same order as the order of requests sent to the scheduler.

Since one dynamic batcher feeds all instances of a model, ordering must hold
across instances. It currently does not.

Root cause

b06d69b ("fix: Response Cache Memory Leak", #449) moved the completion-queue
slot reservation out of DelegateResponse() and into the response delegator:

// before -- reserved at enqueue, in arrival order
std::lock_guard<std::mutex> lock(completion_queue_mtx_);
completion_queue_.emplace_back();
auto queue_slot = &completion_queue_.back();

// after -- created inside the delegator, at completion time
completion_queue_.emplace_back();
auto queue_slot = &completion_queue_.back();
queue_slot->emplace_back(std::move(response), flags);

FinalizeResponses() drains with:

while (!completion_queue_.empty() && !completion_queue_.front().empty()) {

The !front().empty() test is the ordering mechanism: it stalls the drain while
the head slot is reserved but unfilled. Once slots are created already-filled,
front() is never empty, the guard becomes dead code, and the drain simply
re-emits in completion order.

Evidence

Two instances of one model (100 ms and 400 ms delay), 12 requests batched 4+4+4,
preserve_ordering: true confirmed in the server log. From the trace:

Trace ids	Batch	compute span	response sent
1-4	A	100 ms	t0 + 100 ms
5-8	B	400 ms	t0 + 400 ms
9-12	C	100 ms	t0 + 200 ms

Requests 9-12 reached the scheduler after 5-8 but were answered ~200 ms earlier.
INFER_RESPONSE_COMPLETE is captured in the frontend response callback
(server/src/http_server.cc), so it reflects response-send time.

qa/L0_batcher/verify_timestamps.py -p requires at least 4 responses after the
slow batch and measures 0.

Fix

Reserve the slot in DelegateResponse() again, but only when
preserve_ordering_ is set.

The leak b06d69b fixed came from reserving unconditionally: with only the
response cache enabled, the delegator takes the else branch and sends
directly, so the slot was never filled and never popped, growing
completion_queue_ for the lifetime of the model. Guarding the reservation
keeps that path allocation-free and restores ordering.

std::deque does not invalidate references to existing elements on insertion at
either end, and a slot is only pop_fronted after its FINAL flag is seen,
after which the delegator cannot fire again -- the invariant the original code
relied on.

Test coverage

qa/L0_batcher test_multi_batch_preserve_ordering covers this. It has not run
in CI since server 838966ae0 (2024-12-16), which gated it on TEST_WINDOWS
without initialising the variable on Linux, so [ $TEST_WINDOWS -eq 0 ] errored
and took the else branch. With that gate repaired, the test fails deterministically
on 10 GPU platforms (A100, A30, H100, B200, GB200, GB300, RTX50, DGX-Spark,
AGX-Thor, IGX-Orin) across RHEL and Ubuntu, in plain, _shm and _cudashm
variants -- 23 CI jobs. It also reproduced on a same-job retry.

Validation status

• clang-format (v16.0.5) and codespell pass
• Not yet built or run. The fix is derived from the diff and the
  surrounding invariants. L0_batcher test_multi_batch_preserve_ordering
  going green is the real proof and still needs a CI run.

[greptile-apps]

Greptile Summary

This PR moves preserve-ordering completion-slot reservation from response completion to DelegateResponse. The change restores ordering across concurrently completing model instances, but reservation still occurs too late to preserve scheduler arrival order when cache hits bypass queued misses.

• Reserves an empty completion-queue slot when response delegation is configured.
• Captures that slot in the delegator and fills it when responses complete.
• Avoids reserving slots when only response caching is enabled.

Confidence Score: 4/5

This PR should not merge until completion slots are reserved at a scheduler-wide point that preserves arrival order across both cache-hit and queued-request paths.

An earlier cache miss can enter the dynamic-batching queue without a slot while a later cache hit reserves one immediately, leaving the completion queue ordered differently from scheduler arrival order.

Files Needing Attention: src/dynamic_batch_scheduler.cc

Important Files Changed

Filename	Overview
src/dynamic_batch_scheduler.cc	Moves completion-slot reservation into DelegateResponse, restoring cross-instance completion ordering but allowing later cache hits to reserve ahead of earlier queued misses.

Sequence Diagram

sequenceDiagram
    participant R1 as Earlier cache miss
    participant E as Enqueue
    participant B as BatcherThread
    participant Q as completion_queue
    participant R2 as Later cache hit
    R1->>E: Arrives first
    E->>B: Queue request (no slot reserved)
    R2->>E: Arrives later and hits cache
    E->>Q: Reserve slot 0 for R2
    B->>Q: Later reserve slot 1 for R1
    Note over Q: Queue order is R2, R1 rather than arrival order R1, R2

Loading

_{Reviews (1): Last reviewed commit: "fix: reserve completion-queue slot at en..." | Re-trigger Greptile}

[greptile-apps]

Slot reservation breaks arrival order

When an earlier cache miss remains in the dynamic-batching queue and a later request hits the cache, the hit reserves its completion slot immediately while the miss reserves only after BatcherThread dequeues it, causing the later response to occupy an earlier slot and violating scheduler-wide preserve_ordering.

[mc-nv]

Thanks — the mechanism described is accurate, and worth recording. Two points on scope.

The interleaving gap is pre-existing, not introduced here.

DelegateResponse() is called from three sites, and this PR does not move any of them:

Line	Enclosing function	Slot reserved
228	Enqueue() — cache hit	at arrival
242	Enqueue() — dynamic batching disabled	at arrival
395	BatcherThread() — dynamic batching	at dequeue

Those same three sites, at the same lines and in the same functions, exist in the
code prior to b06d69b — the design this PR restores:

pre-b06d69b: line 228 inside Enqueue()
pre-b06d69b: line 242 inside Enqueue()
pre-b06d69b: line 395 inside BatcherThread()

So a later cache hit reserving ahead of an earlier queued miss was possible before
b06d69b, is possible on the current branch, and remains possible after this PR.
Nothing in ValidateModelConfig prevents response_cache and preserve_ordering
from being enabled together, so the scenario is reachable — it is just not new.

What each state actually does:

	all dynamic-batching	cache hit vs. queued miss
pre-b06d69b	ordered	unordered
current branch	unordered	unordered
this PR	ordered	unordered

The regression this PR targets is the middle row: with slots created at completion
time, FinalizeResponses()'s !completion_queue_.front().empty() stall guard can
never fire, so ordering is lost for every model using preserve_ordering,
cache or no cache. That reproduces deterministically across 10 GPU platforms on
both RHEL and Ubuntu, and on a same-job retry.

Holding this fix until the cache-hit path is also addressed would mean shipping
with preserve_ordering non-functional for all models in order to avoid shipping
it non-functional for cache-enabled ones. I would rather land the narrow
restoration and track the remaining gap separately — the proper fix there is
reserving at a single scheduler-wide point covering all three paths, which is a
larger change than a release-branch fix should carry.

I will open a separate issue for the cache-hit/queued-miss ordering gap and link it
here.

Validation status: unchanged from the description — CI is in flight and this is
not yet proven green. L0_batcher test_multi_batch_preserve_ordering going from
responses after large delay count: 0 to >= 4 is the acceptance criterion, and I
will not ask for merge before that lands.