[aeron-gh]

What type of PR is this?

/kind documentation

What this PR does / why we need it:

Adds the design proposal for the OpenAI-compatible Batch API discussed in #1291, under docs/proposal/.

Neither vLLM nor SGLang serves /v1/files or /v1/batches on its online server (vLLM's batch mode is the offline vllm run-batch, which starts its own engine, and its /v1/chat/completions/batch is a single synchronous request), so Kthena owns the batch lifecycle and sends each input line through the router's own request path.

What the proposal covers:

• API: OpenAI-compatible /v1/files and /v1/batches, the same object shapes, line format, status flow and error envelope, with every difference listed.
• State: job records, queue, leases and checkpoints in Redis; file bytes behind a FileStore interface, first backed by a shared volume.
• Ownership: a renewable lease and a random fence token, so one replica owns a batch and a partitioned owner cannot corrupt the new owner's work.
• Resumption: per-owner result segments with a valid-byte prefix, plus a done bitmap, so every custom_id is answered exactly once in the committed output across a takeover.
• Dispatch: each line goes through the router's handler chain in an engine that binds no listener, so route matching, rate limiting, fairness and the KV-cache aware plugins all apply, and the identity cannot be forged from the network.
• Failures and limits: temporary failures retry instead of failing the line, and per-batch and per-tenant storage limits plus a sweeper keep one batch from filling the volume.
• Operations: off by default, Helm values, an optional worker pool, readiness that never affects interactive traffic, cleanup, metrics, and a test plan.

The numbers in it come from a working proof of concept: 20k-request batches complete with 0 missing and 0 duplicated custom_id across a forced pod kill, and router memory stays at 15-40 MiB at 50k requests. Testing the proof of concept against this design found gaps, and the proposal lists each one with what the design does instead.

Which issue(s) this PR fixes:

Part of #1291

Bug evidence (required for bug-related PRs):

N/A

Special notes for your reviewer:

Documentation only, no code. Once the design is agreed I'll send the implementation as small serial PRs, in the order under Implementation Plan.

Five questions are open, each with a recommendation, under Decisions needed: the dispatch hook, how batch lines count against rate limits and fairness, the Redis persistence requirement, session stickiness, and one model per input file.

Does this PR introduce a user-facing change?:

NONE

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign git-malu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• docs/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[aeron-gh]

@YaoZengzeng @kube-gopher
PTAL

[YaoZengzeng]

Read the full proposal and verified the codebase claims (the /v1/*path catch-all at cmd/kthena-router/app/router.go:296, common.UserIdKey, fairness keying). The overall shape — lease + fence, per-ownership segments with valid-byte prefixes, done bitmap, in-process dispatch — is sound, and the PoC evidence is appreciated. Left a few design questions inline; the main one is how the context-carried identity bridges into the gin middleware chain, since fairness/rate-limiting/access-log all key off gin context state set by middleware.

[YaoZengzeng]

Could the proposal specify exactly where in the chain a dispatched line enters? Today /v1 requests flow through AccessLogMiddleware and AuthMiddleware before router.HandlerFunc() (cmd/kthena-router/app/router.go:293-296), and per-user rate limiting / fairness read c.GetString(common.UserIdKey), which is set only by the auth middleware from the verified token (filters/auth/authentication.go:306).

• If batch dispatch enters at HandlerFunc() directly, UserIdKey is never set, so batch lines run with an empty user — the "same rate limits, the same fairness queue, the same access log" claim wouldn't hold.
• If it goes through the full gin engine, the auth middleware will reject lines that carry no credentials, so it must be taught to trust the context-carried identity — a security-sensitive change that deserves to be spelled out here (which code checks the context, and how it's kept unreachable from real network requests).

Either answer is fine, but the bridging mechanism is the crux of this design decision and should be explicit in the doc.

[aeron-gh]

doc didn't say, my bad. lines go through a second gin engine, not the network one -- recovery -> batchIdentity -> handler, never bound to a listener. batchIdentity stands in for AuthMiddleware: reads identity off the request context, 403 if missing, sets UserIdKey + gateway keys. real AuthMiddleware untouched. written up under Dispatch now

[YaoZengzeng]

Story 2 says "enable the batch worker on two of six router replicas", but the chart renders a single router Deployment, so worker.enabled applies to all replicas uniformly. How does an operator actually achieve the two-of-six split — a second Deployment / separate router pool with different values? Worth stating, since the isolation story (and the per-replica p99 numbers) leans on this capability.

[aeron-gh]

yeahh right, one Deployment so it's uniform. story 2 now says two router pools sharing one redis + volume, and Configuration spells out that the split needs a second chart release.

[YaoZengzeng]

The queue is FIFO by enqueue time with no per-tenant limits: one tenant queuing many 50k-line batches starves everyone else, and nothing bounds the total number of queued jobs. Consider per-tenant caps (max queued / max active batches) in the limits block — OpenAI enforces per-org batch queue limits for the same reason. Fine as a stated follow-up, but worth a sentence now.

[aeron-gh]

okiee agreed, nothing bounds it today. added maxQueuedBatchesPerTenant/maxActiveBatchesPerTenant as not-built plus a risks row. per-tenant index already exists, so it's really just the admission check on create and skipping a capped tenant in the claim loop.

[YaoZengzeng]

What happens when a response exceeds maxResponseBytes? batchWriter "collects the body up to a cap" — silent truncation would produce an invalid JSON line in the output file. Presumably the line should be recorded in the error file instead; worth specifying.

[aeron-gh]

it's not truncated -- the line is marked failed and written to the error file as response_too_large, counted in request_counts.failed. doc didn't say so though..
added

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[aeron-gh]

@YaoZengzeng PTAL

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[aeron-gh]

@YaoZengzeng PTAL

[YaoZengzeng]

Field not fully displayed

[aeron-gh]

okk done, labels are single line now and the detail moved into a small table under the diagram.

[YaoZengzeng]

I'm not entirely clear on the meaning of this part.

[aeron-gh]

done, rewrote it

[YaoZengzeng]

This Design Details is too abstract.

I would like to see the specific steps the Router takes to handle Batch Requests,

how the various components work together,

what new components are added,

and how they are integrated with existing components.

[aeron-gh]

yepp, it was organised by concept and never said what actually gets built. added three sections at the top of Design Details: -->

• what is added: file by file, taken off the poc branch rather than invented
• how it plugs into the existing router: the only existing files touched. the go side is 29 added lines across two of them, no deletions anywhere
• end to end: sequence diagram plus 8 numbered steps, each naming the component
  that does it

pushed as 06eefbc.

[Copilot]

🟡 Changes recommended

Unresolved critical issues affect fencing, error compatibility, durability, duplicate IDs, and JWT identity restoration.

5 open findings

• Duplicate custom IDs violate exactly-once guarantees · New
• RDB snapshots cannot guarantee durable checkpoint recovery · New
• Error schema and authentication failures lack parity · New
• Lease fencing does not prevent stale backend dispatches · New
• Batch identity omits JWT token state · New

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

Multiple critical and moderate design and operational issues remain unresolved.

7 open findings

• File admission and deletion have a race causing missing inputs · New
• Checkpoint durability is not guaranteed before acknowledgment · New
• Non-idempotent file publication can duplicate committed output · New
• Recommended io.ReadSeekCloser interface does not exist in Go · New
• Empty storage claim allows non-persistent batch storage · New
• Writability checks do not enforce a shared RWX volume · New
• Batch identity omits JWT token state

4 resolved since last review

• Lease fencing does not prevent stale backend dispatches
• Error schema and authentication failures lack parity
• RDB snapshots cannot guarantee durable checkpoint recovery
• Duplicate custom IDs violate exactly-once guarantees

Previously missed (1)

In code that hasn't changed since last review

Separate router pools do not isolate model backend capacity

docs/​proposal/​kthena-router-batch-api.md:162

Separate router pools do not isolate the model backends. Both deployments still route to the same ModelServers, and this proposal says adaptive back-off is not yet built; batch requests can therefore consume backend capacity and raise interactive latency. The worker switch isolates router-side CPU only, so this story cannot promise an unaffected interactive p99 without backend partitioning or priority/adaptive flow control.

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

Critical ownership, fencing, storage, security, and compatibility issues remain unresolved.

11 open findings

• Final output publishing is not fence-safe or idempotent · New
• Redis everysec persistence can reuse fencing epochs · New
• Header-based routing lacks a safe OpenAI JSONL source · New
• Writability checks do not enforce a shared RWX volume
• Empty storage claim allows non-persistent batch storage
• Recommended io.ReadSeekCloser interface does not exist in Go
• Non-idempotent file publication can duplicate committed output
• Checkpoint durability is not guaranteed before acknowledgment
• File admission and deletion have a race causing missing inputs
• Batch identity omits JWT token state
• Per-process limiters do not enforce cross-replica fairness · New

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

Unresolved API compatibility, storage safety, fencing, lifecycle, and capacity issues remain.

5 open findings

• Add total output and disk capacity limits · New
• Redis everysec persistence can reuse fencing epochs
• Writability checks do not enforce a shared RWX volume
• Batch identity omits JWT token state
• Per-process limiters do not enforce cross-replica fairness

7 resolved since last review

• Header-based routing lacks a safe OpenAI JSONL source
• Final output publishing is not fence-safe or idempotent
• Empty storage claim allows non-persistent batch storage
• Recommended io.ReadSeekCloser interface does not exist in Go
• Non-idempotent file publication can duplicate committed output
• Checkpoint durability is not guaranteed before acknowledgment
• File admission and deletion have a race causing missing inputs

Previously missed (2)

In code that hasn't changed since last review

Add a configurable Redis installation prefix

docs/​proposal/​kthena-router-batch-api.md:409

The statement that all Redis keys are prefixed is misleading: the key table includes generic queue, active, and fence keys and unscoped type prefixes. Since Redis is optional today and can be shared by multiple router installations, an operator can cause batch state to collide with another deployment; specify a configurable installation prefix (while keeping the same prefix across the intentional API/worker pools).

Require atomic input reference acquisition

docs/​proposal/​kthena-router-batch-api.md:565

This race is acknowledged but left as an optional follow-up even though it violates the stated invariant that a delete keeps input bytes alive until batch creation is complete. A delete can remove the bytes between validation and fileref insertion, so atomic reference acquisition (or the documented recheck-and-release protocol) needs to be a required part of batch creation before implementation proceeds.

🧠 Review effort: Lite

[aeron-gh]

@YaoZengzeng
one more from going through this: batchIdentity restores UserIdKey and the gateway keys but not common.JWTTokenKey, so a ModelServer with sessionKeySource JWTClaim resolves to an empty session key on batch lines
(extract.go:77 -> ClaimFromContext). replaying the raw token would put a bearer credential in redis for the life of the batch and it can expire mid-run, so my instinct is to resolve the session key once at create time and replay just that string.
written up under Dispatch as a flagged decision. does that direction look right, or would you rather phase 1 declares JWTClaim affinity unsupported for batch?

[Copilot]

🟡 Changes recommended

Critical unresolved issues remain in batch endpoint validation and failed-batch result completeness, alongside additional correctness, quota, worker-configuration, and volume-initialization gaps.

6 open issues

• Restrict batch dispatch to the supported model endpoint · New
• Emit error records for every unfinished line on failure · New
• Exclude the persistent volume identity marker from orphan cleanup · New
• Reserve transient storage atomically to enforce tenant quotas · New
• Require a worker when batch processing is enabled · New
• Make volume identity initialization atomic across replicas · New

5 resolved since last review

• Add total output and disk capacity limits
• Redis everysec persistence can reuse fencing epochs
• Writability checks do not enforce a shared RWX volume
• Batch identity omits JWT token state
• Per-process limiters do not enforce cross-replica fairness

_{Review effort: Lite}

[aeron-gh]

@YaoZengzeng pushed an update, it covers your comments and the open bot findings now. ptal when you get a chance ta!!
copilot left 6 more on the last push, all small edge cases (endpoint check, failed batches, volume-id, tenant quota, worker config). i have fixes ready but holding them so the doc doesn't keep growing, it's already big. happy to push them if you want