QVAC-22704 feat[api]: multi-job continuous batching in the SDK - #3682
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK runtime to support multi-job continuous batching for llama.cpp models by admitting multiple concurrent completion() / batchCompletion() requests per model (up to that model’s configured parallel), and by switching completionStream cancellation to a per-request (job-scoped) cancel path so cancelling one run doesn’t abort its peers.
Changes:
- Add per-request overrides in the request registry (
maxConcurrentPerModel,slotGroup,onOverflow) to enforce per-model concurrency and specialized serialization lanes. - Update llama.cpp completion/batch handlers to (a) admit requests up to the model’s
parallel, (b) reject overflow on single-slot models, (c) serialize disk KV-cache writes on a dedicated lane, and (d) route cancellation through response-level cancel for per-request completion cancels. - Update unit/bare/e2e tests and docs, bump
@qvac/llm-llamacppto^0.40.0, and add an example demonstrating concurrent completions + isolated cancellation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/test/unit/runtime/request-registry.test.ts | Adds/updates registry tests for per-request caps, lanes, and overflow behavior. |
| packages/sdk/test/bare/plugin-cancel-capability.test.ts | Updates cancel capability truth table for request-scoped completion cancels. |
| packages/sdk/server/bare/runtime/request-registry.ts | Implements per-request overrides for concurrency cap/lane/overflow in admission gating. |
| packages/sdk/server/bare/runtime/request-registry-singleton.ts | Updates default policies for shared completion/batch lane and adds cached KV serialization lane constant. |
| packages/sdk/server/bare/runtime/index.ts | Re-exports the cached-lane constant from the runtime entrypoint. |
| packages/sdk/server/bare/registry/model-registry.ts | Extends the run response interface to include a cancel() method. |
| packages/sdk/server/bare/plugins/llamacpp-completion/plugin.ts | Uses per-model parallel for admission, configures addon overflow behavior, and switches completionStream cancel scope to request. |
| packages/sdk/server/bare/plugins/llamacpp-completion/ops/completion-stream.ts | Routes abort to response.cancel() (job-scoped) and registers admitted responses for targeted cancellation. |
| packages/sdk/server/bare/plugins/llamacpp-completion/ops/batch-completion-stream.ts | Routes abort through batch response cancel (still model-scoped today, ready for future per-batch cancel handle). |
| packages/sdk/package.json | Bumps @qvac/llm-llamacpp dependency to ^0.40.0. |
| packages/sdk/examples/multi-job-completion.ts | Adds a runnable example showing concurrent completions and cancelling one by requestId. |
| packages/sdk/e2e/tests/shared/executors/completion-executor.ts | Updates E2E executor expectations for single-slot rejection contract. |
| packages/sdk/e2e/tests/shared/executors/cancellation-executor.ts | Updates cancellation E2E to reflect single-slot rejection + admitted run success. |
| .cursor/rules/sdk/request-lifecycle-primitives.mdc | Updates internal SDK lifecycle docs for request-scoped cancels and per-request registry overrides. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ecfdb14 to
d159ec1
Compare
License compliance — cleanNo new dependency license findings in this PR. Warn-only (shadow) mode — this check does not block merges yet. Updated automatically by the canonical license compliance workflow. NOTICE presence (advisory)Missing NOTICE (advisory, does not block):
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/sdk/server/bare/runtime/request-registry.ts:482
maxConcurrentPerModelis treated as an arbitrary finite number, so non-integers can accidentally over-admit. For example, if a handler passesparallel: 1.5, the gate will admit 2 requests becauseactiveis an integer and the check isactive < maxConcurrent. Since modelparallelis effectively a slot count, it should be clamped to an integer >= 1 here.
// The per-request override (the model's own `parallel`) wins over the
// per-kind default. A non-finite effective cap disables gating entirely; a
// finite value is floored at 1 (matching `normalizePolicy`) so a stray 0
// can't wedge the lane into queuing every request forever.
const requestedMax = opts.maxConcurrentPerModel ?? policy.maxConcurrent
if (!Number.isFinite(requestedMax)) {
return { slotKey: undefined }
}
const maxConcurrent = requestedMax < 1 ? 1 : requestedMax
packages/sdk/server/bare/plugins/llamacpp-completion/plugin.ts:59
getModelParallel()currently returnsNumber(config.parallel) || 1, which allows negative numbers and non-integers (e.g.1.5). Because the request registry treats the cap as a numeric threshold, a non-integer can result in admitting more jobs than intended. Clamping to an integer >= 1 also aligns with the comment (“slot count”).
// The model's concurrent sequence slots. Missing / 0 / NaN all mean single-slot.
function getModelParallel(config: { parallel?: number | undefined }) {
return Number(config.parallel) || 1
}
.cursor/rules/sdk/request-lifecycle-primitives.mdc:220
- The truth-table row for
llamacpp-completion.batchCompletionStreamstill lists{ scope: "model" }, but the handler now declares per-request cancel ({ scope: "request", hard: true }). Update the table to match the code and the pinned test.
| Plugin | Handler(s) | `cancel` |
|------------------------------|---------------------------------------|-----------------------------------|
| `llamacpp-completion` | `completionStream` | `{ scope: "request", hard: true }`|
| `llamacpp-completion` | `batchCompletionStream` | `{ scope: "model", hard: true }` |
| `llamacpp-completion` | `translate` | `{ scope: "model", hard: true }` |
| `llamacpp-completion` | `finetune` | `{ scope: "model", hard: true }` |
.cursor/rules/sdk/request-lifecycle-primitives.mdc:203
- This doc still says
batchCompletionStreamuses model-scoped cancel “pending a per-batch addon handle”, but the plugin now declaresbatchCompletionStream: { scope: "request", hard: true }and the truth-table test was updated accordingly. The bullet should be generic and not single outbatchCompletionStreamanymore.
This issue also appears on line 215 of the same file.
`cancel.scope`:
- `"request"` — addon targets a specific in-flight request by id. Framework can route `cancel({ requestId })` straight to the addon without colliding with siblings on the same model.
- `"model"` — addon cancels "whatever is currently running on this model" (`addon.cancel()` semantics). Under continuous batching this also stops concurrent peers on the same model, so it is a coarse fallback, not a per-request cancel. `batchCompletionStream` still uses it, pending a per-batch addon handle.
- `"none"` — addon does not expose a cancel surface. SDK falls back to soft-cancel (stop yielding, drop result, skip post-processing; C++ work runs to completion).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/sdk/server/bare/runtime/request-registry.ts:482
maxConcurrentPerModelcan be a non-integer (e.g. configparallel: 1.5or2.3). With the current comparison (st.active < maxConcurrent) this effectively rounds up and can admit more concurrent requests than intended (e.g. 1.5 admits 2). Coerce the effective cap to an integer (and still clamp to >=1) so admission matches the configured slot count deterministically.
const requestedMax = opts.maxConcurrentPerModel ?? policy.maxConcurrent
if (!Number.isFinite(requestedMax)) {
return { slotKey: undefined }
}
const maxConcurrent = requestedMax < 1 ? 1 : requestedMax
packages/sdk/server/bare/plugins/llamacpp-completion/plugin.ts:59
getModelParallel()currently returns the rawNumber(parallel), which can be negative or non-integer. That value feeds bothrejectWhenBusyand the registry cap, so a config likeparallel: 2.9can accidentally behave like 3 slots andparallel: -1can behave inconsistently. Normalize to a finite integer >= 1 (and treat missing/0/NaN/<1 as 1) to match the semantics described in the comment.
// The model's concurrent sequence slots. Missing / 0 / NaN all mean single-slot.
function getModelParallel(config: { parallel?: number | undefined }) {
return Number(config.parallel) || 1
}
packages/sdk/server/bare/plugins/llamacpp-completion/plugin.ts:393
- When
useCachedLaneis true, the request moves to a differentslotGroupwith its own cap (1), so it no longer contends for the shared completion lane capped atparallel. That makes it possible to admitparallelplain completions plus one cached completion concurrently, pushing queueing back into the addon and breaking the PR description’s "one admission pool per model capped at parallel" and FCFS behavior across all completion requests.
requestId: request.requestId ?? generateServerRequestId(),
kind: 'completion',
modelId: request.modelId,
maxConcurrentPerModel: useCachedLane ? 1 : parallel,
...(useCachedLane && { slotGroup: LLAMACPP_COMPLETION_CACHED_SLOT_GROUP })
378b314 to
ca0dd36
Compare
Review StatusCurrent Status: ❌ PENDING Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member. |
573710f to
2c735ff
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Resolve the packages/sdk/package.json addon-pin conflict by fully adopting main's dependency set, so the branch carries no addon-pin divergence from main (the multi-job changes live entirely outside package.json). main already pins tts-ggml 0.7.4, so the earlier pin bump is subsumed; diffusion-cpp stays at main's 0.17.0, whose fabric-10069 / inference-addon-cpp 1.3.3 alignment is a separate follow-up.
A finetune queued behind an active reader (completion / LLM-translate) shares the exclusive lane but is not admitted. pauseFinetune() called the addon-wide model.pause(), whose contract also cancels in-flight inference, so pausing while the finetune was still queued killed that unrelated reader AND left the queued finetune free to start afterwards despite pause returning PAUSED. Track which finetune currently holds the exclusive lane and only route the global model.pause() to that admitted finetune; a merely-queued finetune is cancelled through the registry (nothing has trained yet) and the addon is never touched. Adds a reverse-direction regression test (queued-behind-a-reader pause) that fails on the previous behavior, and updates the handler pause test to admit a finetune first so it still exercises the admitted-pause dispatch.
commitTurn marked the target cache path active before creating its directory and marker, but the outer finally released only the target write lock. A throwing mkdir or markAutoCacheKey therefore leaked the target active-path ref (and any directory/marker created above) for the worker lifetime. Wrap the target setup so a throw releases the active-ref first and prunes the dir/marker before propagating; withCacheStateLock already releases its lock on throw. Adds a setup-failure regression test via a read-only active-path inspection hook.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The earlier pause fix covered a finetune queued behind a reader but missed an admitted finetune with a queued finetune peer: pausing the admitted one freed the exclusive lane and the queued peer then started despite the pause having returned PAUSED. Track finetune requestIds per model (the admitted one plus queued peers). Pausing an admitted finetune now cancels every queued peer first, then pauses the admitted one, so no peer can slip into the lane the pause frees. Adds the full pause state-matrix tests: none, queued-only, admitted-only, and admitted-plus-queued.
kv-cache-auto-concurrency recorded lastTokenAt after the token stream closed, but a cached stream closes only after its post-decode KV commit/rename, so the decode window stretched across the commit — a plain request overlapping only the cached request's commit phase was falsely counted as decode overlap. Record lastTokenAt on every decoded token so the window reflects actual decode.
QVAC E2E —
|
QVAC E2E —
|
Replace the multi-peer finetune tracking with a one-finetune-per-model contract: a second start/resume is rejected while one is pending or running. With no queued finetune peers, pause and cancel never disambiguate multiple finetunes, and a finetune that arrives mid-pause is simply rejected because the pausing one still occupies the model until it unwinds. This removes the peer-cancellation, duplicate-requestId, and arrival-during-pause race class entirely. pauseFinetune now validates the model on every path (ModelNotFoundError for an unloaded model), pauses an admitted finetune through model.pause(), cancels a finetune still queued behind a reader (CANCELLED — nothing has trained), and is a no-op otherwise. Tests cover the full state matrix and invalid inputs: reject-second (while running and while pausing), pause of none / admitted / queued-behind-reader, unloaded model, and revalidation after a lane wait behind a reader.
… docs Require both engine sequence co-residency (avgConcurrentSeq > 1) and overlapping content-token windows in the concurrency-proof executor and the multi-job example — either signal alone can mislead (prefill inflates avgConcurrentSeq; transport buffering can overlap client windows). Tighten the translate broad-cancel e2e to count only tokens received after the cancel acknowledgement. Clarify in the registry BeginOpts comment and the request-lifecycle rules that maxConcurrentPerModel is a top-level request-permit cap — it counts requests, not native sequences.
…indows The client-observed content-token-window overlap is transport-buffering sensitive, so gating on it can flake even when the server genuinely decoded concurrently. Gate only on the engine avgConcurrentSeq (authoritative for native sequence co-residency) and keep the token-window overlap as a reported diagnostic.
Two gaps in the one-finetune-per-model pause path:
- Pause cleared the occupancy only when handle.await() settled, which can precede
model.pause()'s own resolution, letting a new finetune slip in mid-pause. Hold a
pausingModels barrier across model.pause() so a new start/resume is rejected until
the native pause settles, not just until the handle resolves.
- Cancelling a queued finetune used a broad cancel({ modelId, kind }), which misses a
request caught in the registry's grant-to-register handoff window (present in
neither the wait queue nor the active set) — the finetune could then start after
pause reported CANCELLED. Track the single finetune's requestId and cancel by it, so
the cancel-before-begin tripwire covers that window; cancelFinetune uses the same
targeted path.
Adds an adversarial pause-ordering test (the handle settles before the native pause)
that fails without the barrier.
🎯 What problem does this PR solve?
completion()/batchCompletion()calls serialized even on models loaded withparallel > 1, leaving the addon's continuous-batching throughput unreachable from the SDK.📝 How does it solve it?
parallelvalue. Single and batch requests compete first-come-first-serve for admission permits in that lane — one permit per request — and the addon then schedules the actual native sequence slots. Admission bounds concurrent requests, not native sequences: a multi-prompt batch takes one permit but can occupy several native slots, and a same-key cache-lock waiter holds a permit while doing no native work. (Tightening admission to weighted, slot-accurate accounting is a tracked follow-up.)parallelconfig rather than a single global number.parallel = 1or unset) still admits one request at a time and queues the rest — unchanged from before. An N-way model admits up toparallelconcurrently and queues the surplus.cancelJob), leaving concurrent peers on the same model decoding. BothcompletionStreamandbatchCompletionStreamadvertisecancel: { scope: 'request' }.paralleland keep their first-come-first-serve place. Same-file write safety comes from a per-cache-path lock inside the KV-cache session, held across the turn: two turns writing the same cache file serialize, while turns on different cache keys still decode concurrently. Atparallel = 1everything is already serial.🧪 How was it tested?
CI run: https://github.com/tetherto/qvac/actions/runs/32268049857?pr=3682
@qvac/llm-llamacpp@0.44.0types.completion+kv-cache+cancellation) via the MQTT harness against the published addon.completion-concurrent-overlapfires four independent completions on aparallel:4model, requires engineavgConcurrentSeq > 1, and reports client decode-interval overlap as supporting diagnostics.kv-cache-concurrent-same-keyproves same-file serialization.kv-cache-auto-concurrencyseparately requires cached-vs-cached overlap and mixed cached/plain overlap.cancel-isolates-concurrent-batchesrequires the survivor batch to make observable progress after the cancelled batch's acknowledgement.examples/multi-job-completion.tsfires concurrent completions on aparallel > 1model and cancels one by its request id while its peers decode to a clean finish.🔌 API Changes
Additive API surface with no schema removals or signature breaks. Runtime admission and cache-administration behavior changes as described below.
A model loaded with
parallel = 1(or unset) behaves exactly as before: same-model requests serialize FIFO.Cache deletion now rejects key/model combinations that resolve to the cache root or broaden a targeted model deletion to the whole key directory. Omit
modelIdintentionally to delete every model under a key, or use{ all: true }for a full cache wipe.New behavior for LLM translate and finetune: this PR routes them into the shared llama.cpp completion lane, so they are now subject to that lane's per-model bounded wait queue. The 64-deep FIFO queue is shared across completion, batch completion, LLM translate, and finetune waiters per model; once 64 are waiting, the 65th queued request is rejected with
RequestRejectedByPolicyError. NMT translate passes no concurrency cap, never enters the lane, and is unaffected.Availability cost of the exclusive-writer finetune: while a finetune runs it holds the shared lane alone, so every completion, batchCompletion, and LLM translate on that model queues behind it for the finetune full duration. The per-model queue is bounded (64) but the wait is not - a queued reader can be blocked for the whole training run with no timeout. This is intentional; a timeout would turn ordinary inference into a new typed failure and needs an owner/QIP-approved duration and policy across every shared-lane kind. Do not share a model between finetune and interactive requests if you need inference responsiveness during training.
🏛️ QIP disposition
QIP triage is positive because this changes a stable public SDK scheduling/cancellation contract and the llama.cpp addon interaction.
One proposal is appropriate: Per-model multi-job admission and cancellation contract, covering request-count versus native-slot accounting, the shared lane, the fixed 64-entry queue, exclusive finetune behavior, and per-request cancellation. KV-cache lock implementation and the localized cancellation/shutdown race fixes do not need separate proposals.
🔗 Dependencies
@qvac/llm-llamacpp@^0.44.0— the multi-job continuous-batching addon line (from QVAC-18397: Multi-job queue at addon-cpp and LLM (Needed for LLM Continuous Batching Optimizations) #3445), matching main's current pin. Published to npm.