Skip to content

feat: batch multiple HTTP requests in one API executor task - #141

Open
timzsu wants to merge 22 commits into
mainfrom
zsu/api-executor-batch
Open

timzsu wants to merge 22 commits into
mainfrom
zsu/api-executor-batch

Conversation

@timzsu

@timzsu timzsu commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Purpose

One API executor task issued exactly one HTTP request, so a multi-row op became N separate FlowMesh tasks. The serving endpoint answers request-by-request, so a batch is N requests — issue them in parallel from one task.

Changes

  • api_executor.pyspec.data is required and there is a single execution path; a one-row batch is just N=1. The invariant request is built once and each row substitutes its prompt into a {{prompt}} slot. Rows are issued on a ThreadPoolExecutor bounded by _MAX_CONCURRENCY = 8, and the httpx pool is sized from the capped concurrency so requests cannot queue on connections.
  • HTTP errors are classified before strict payload parsing, so a 4xx/5xx is reported as an HTTP error rather than as a parse failure — previously a retryable 503 became permanent.
  • Task-scoped cancellation — check-and-reset is atomic under a lock, rechecked before run() and again in flight.
  • APIItem / APIResult.items added to the shared schemas and mirrored in the SDK with the identical alias="json", registered in _RESULT_MODEL_NAMES so the drift guard covers it.
  • n8n_parser.py, api_two_stage.yaml, docs/EXECUTORS.md, docs/WORKFLOWS.md.

Design

One path, not two. No if batching: branch, no second request builder, no second result shape.

Result shape. APIResult is extended rather than given a sibling: the discriminated union keys on task_type and admits one model per tag. Lumilake's result normaliser already reads items before falling back to text, so the consumer needs no change.

Placeholder {{prompt}}, deliberately not ${prompt}. ${...} is the server-side stage-reference syntax resolved at dispatch, before a task reaches a worker; per-row substitution happens worker-side, after that. A ${prompt} token would be walked by the stage resolver looking for a stage named "prompt".

Row alignment. Results are collected keyed by row index and rebuilt in input order, so completion order cannot disturb it. Any row failing fails the whole task; partial success is deliberately not offered.

Cancellation, stated precisely. A request already inside the synchronous HTTP call is NOT aborted. Cancelling prevents queued rows from starting and marks the task cancelled once in-flight requests return. The docstring previously overclaimed this and was corrected.

Test Plan

End-to-end on a stack built from this branch, against the office serving endpoint https://lum.id/llm:

  1. Submit a multi-row API workflow and confirm it is one task.
  2. Confirm each output row answers its own prompt and index == position.
  3. Force a 5xx with raise_for_status: false and confirm the per-row status survives.
  4. Force a 503 and confirm it fails with the concrete orchestrator error, retryable.
  5. Cancel a running task and confirm it reaches cancelled.
  6. Paired A/B at equal N and prompts, varying only spec.api.concurrency, repeated — never a single unpaired timing.

Test Result

All seven ran and passed against https://lum.id/llm with qwen3.8-27b.

  • One task, failed_tasks: [], 8 rows, 8 distinct answers, index == position on every row — row alignment on real HTTP rather than httpx.MockTransport.
  • 5xx preserved per-row status_code: 500; the 503 surfaced API request returned status 503 (row 0): service unavailable, the orchestrator's own error and not a batch wrapper.
  • Cancellation reached cancelled tasks on the apibatch server.
  • Parallelism, paired at N=8: serial median 6.44s (range 6.43–7.75), parallel median 2.66s (range 2.35–4.12). Ranges do not overlap. An earlier one-shot pair had given 5.89s serial against 8.23s parallel — a cold-start artefact that reversed under repetition, which is why the pairing is repeated rather than single.

Pre-submission Checklist
  • I have read the contribution guidelines.
  • I have run pre-commit run --all-files and fixed any issues.
  • I have added or updated tests covering my changes (if applicable).
  • I have verified that uv run pytest tests/ passes locally.
  • If I changed shared schemas or proto definitions, I have checked downstream compatibility across Server and Worker.
  • If I changed the SDK or CLI, I have verified the affected packages work (uv sync --all-packages --group ci --frozen).
  • If this is a breaking change, I have prefixed the PR title with [BREAKING] and described migration steps above.
  • I have updated documentation or config examples if user-facing behavior changed.

@timzsu
timzsu added this pull request to stack #142 September 18, 2026 04:47
@timzsu timzsu changed the title feat(worker): batch multiple HTTP requests in one API executor task feat: batch multiple HTTP requests in one API executor task Sep 18, 2026
@timzsu
timzsu force-pushed the zsu/api-executor-batch branch from 5a28791 to 59ee155 Compare September 18, 2026 05:26
Base automatically changed from api-endpoint-agnostic to main September 18, 2026 12:05
@timzsu
timzsu force-pushed the zsu/api-executor-batch branch from 4382893 to 178275e Compare September 18, 2026 12:05
timzsu and others added 20 commits September 19, 2026 14:47
When spec.data is present, the API executor issues one request per row,
substituting each row's prompt for the {{prompt}} placeholder in the
request body, and returns the responses row-aligned in APIResult.items.
The single-request path is unchanged. Reuses the DataMixin parsing infra
shared with the vLLM executor.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
The batch change added `items: list[APIItem]` to the server-side APIResult
without the SDK counterpart, so tests/sdk/test_schema_compat.py failed with
"APIResult missing server fields: ['items']".

The SDK mirrors every server result model and a compat test enforces that
they stay in step. This adds APIItem to the SDK payloads beside the existing
InferenceItem and Omni* items, carrying the identical `alias="json"` so the
wire-alias check passes too, adds `items` to the SDK APIResult, and registers
APIItem in _RESULT_MODEL_NAMES so the drift guard covers it from now on
rather than only the field that happened to break.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Require spec.data (as the vLLM executor does) and issue one request per
row in parallel, returning row-aligned APIResult.items. A single request
is a one-row spec.data. The request skeleton is built once and each worker
only substitutes its prompt into the prepared body. Concurrency is bounded
by spec.api.concurrency (default 8) and the client connection pool is sized
to it. Migrate api_two_stage.yaml and the n8n parser to carry spec.data.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Remove the separate api_batch.yaml template and make api_two_stage.yaml
demonstrate batching: multiple rows fan out through both stages, each row's
prompt fills the {{prompt}} worker-side slot, and stage two consumes stage
one. A single request is a one-row spec.data, so a dedicated batch template
no longer represents a distinct feature.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
The committed APIItem could not round-trip its own output. The executor
constructs the item by field name (response_json=...), which the json
alias plus extra="forbid" rejects. The worker serialises results with
model_dump_json() and no by_alias, so it emits the field name
response_json; the server then re-validates against the json alias and
rejects it. Every API-executor result would 422 on ingest.

populate_by_name=True fixes construction and ingest while keeping the
json alias accepted on input, so it is backward compatible in both
directions.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
The connection pool was hard-coded at 8 while the ThreadPoolExecutor used
the effective spec.api.concurrency, and the client cache key omitted
concurrency so a pool built for one value was reused for another. Size both
limits from the capped configured value and include concurrency in the cache
key.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
APIResult is batch-only: text lives per row, so a dependent stage's
placeholder must address items.0.text rather than a scalar text field that
is never populated. Update the n8n parser, the two-stage example, and add
an end-to-end dependent-stage resolution test.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
APIResult is batch-only, so the docstring no longer describes a scalar
single-request path. Add an SDK-side by-name construct/serialize/revalidate
test so the populate_by_name setting is guarded on both sides of the wire.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Replace the added inline comments with condensed docstring notes so the
non-obvious decisions (batch-only APIResult row addressing, the client cache
key and pool sizing) stay documented without inline comment blocks.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Move the module-scope nvmlInit() call in the GPU cleanup test behind a
skip guard so importing the module is side-effect free on GPU-less hosts,
including CI. Replace the inline import and inline type: ignore comments
with top-level imports and a cast, correct the pool-sizing comment to say
the pool is sized to the effective concurrency (capped), and assert the
issued requests as an unordered collection instead of assuming thread-pool
start order.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
…rral

Give the recording transport distinct status, usage, and headers per row
(with raise_for_status false and include_headers true) and assert them per
row so a mix-up in those fields is caught. Add a run-level test at an
uncapped concurrency (1 and 4) that isolates the effective value forwarded
to the client, and a test that observes concurrency: 1 serializing requests.
Defer nvmlInit() to a module fixture so importing the GPU test is
side-effect free, and add an integrated translated-n8n-to-stage-resolution
test covering the items.0.text production change end to end.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
A 5xx error body (e.g. a 503 {"error": ...}) has no OpenAI usage or
choices, so parsing it before the retryable classification raised a
non-retryable ExecutionError and a retryable 503 was permanently failed.
Classify HTTP errors first, and accept absent usage/text in the success
path so a no-usage 2xx or a 5xx under raise_for_status: false still
produces a row-aligned item.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
The executor inherited the no-op Executor.cancel(), so an interrupt during a
batch let queued rows continue and the batch could report success. Add a
per-instance cancel event, set it from cancel(), and check it before issuing
each request and before submitting queued futures, raising TaskCancelledError
so the runner aborts the batch.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
The SDK by-name round-trip test only validates and revalidates the SDK
model, so its docstring no longer claims a cross-package worker/server
boundary. Narrow the NVML fixture's skip to pynvml.NVMLError so a real
initialization bug is not swallowed as a missing GPU.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
A cancel landing between run()'s pre-run check and its event reset was
dropped. Hold a lock across the check-and-clear and in cancel() so no
interval exists where a cancel is accepted and then discarded. Document
the capped concurrency and cancellation behavior in WORKFLOWS.md.

Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
The docs claimed in-flight requests are aborted on cancel; they are not -
a request inside the synchronous HTTP call is not interrupted. Say that
cancellation prevents queued rows and marks the task cancelled once
in-flight requests return. Rename the collection-guard test to describe
what it actually asserts.

Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
@timzsu
timzsu force-pushed the zsu/api-executor-batch branch from e0418e9 to cef69ac Compare September 19, 2026 06:47
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
@timzsu
timzsu marked this pull request as ready for review September 19, 2026 07:07
@timzsu
timzsu requested a review from kaiitunnz as a code owner September 19, 2026 07:07
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.

1 participant