Skip to content

feat: user-specified retries for transient API failures - #155

Merged
timzsu merged 8 commits into
mainfrom
zsu/api-executor-retry
Sep 27, 2026
Merged

timzsu merged 8 commits into
mainfrom
zsu/api-executor-retry

Conversation

@timzsu

@timzsu timzsu commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Purpose

The API executor marks transient failures (5xx, 408, 429, connection errors) as retryable=True but never retried them — a request failed immediately on the first transient error. In production the LLM serving endpoint intermittently returns 504 Gateway Timeout on large requests; the same request succeeds when retried. A user-specified retry count lets the executor re-issue and succeed.

Changes

  • spec.api.retries (new field, default 0 preserving current behavior) — how many times a transient failure is re-issued before the task fails.
  • _request_with_retries — wraps the request in a retry loop with a fixed 1s backoff between attempts. A retryable failure (connection error, 5xx, 408, 429) retries up to retries times; non-retryable 4xx and cancelled tasks stop immediately.
  • Cancellation during a retry. The backoff waits on the task's cancel event, so a cancel ends pending retries at once and the task reports cancelled; a request already inside its HTTP call is not interrupted.
  • Cancellation is scoped to one task. A worker reuses one API executor across tasks, so the executor records which task it is running: a cancel addressed to another task is ignored, and a cancel that arrives before its task starts still takes effect when that task starts. Before this, a cancelled executor stayed cancelled for the next task it ran.
  • Tests covering retries-exhausted (fails loudly), retry-succeeds, cancel during backoff, a late cancel for a finished task, and a cancel before its task starts; docs updated.

Design

The retry is per-request, preserving the executor's concurrency model. The final attempt's failure propagates to the caller — a request that exhausts retries still fails loudly, never silently swallowed.

Test Plan

The repo's own pre-commit gate over every changed file, and the full suite on the repo's interpreter.

Test Result

E2E against a live serving endpoint. A FlowMesh worker running this change serves Lumilake's paper-ingestion runs against qwen3.8-27b with spec.api.retries: 3. Its per-call log shows both outcomes:

  • calls whose first attempt failed were re-issued and returned 200 on the second attempt, in single-request tasks and in multi-row batch tasks, where only the failed row was re-issued;
  • a task whose endpoint kept returning 503 made four attempts per row, the first plus three retries, and then failed with the 503 rather than reporting success.

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 force-pushed the zsu/api-executor-retry branch from a8cc9c6 to 04639c2 Compare September 23, 2026 14:54
@timzsu
timzsu marked this pull request as ready for review September 23, 2026 14:57
@timzsu
timzsu requested a review from kaiitunnz as a code owner September 23, 2026 14:57
@timzsu
timzsu added this pull request to stack #156 September 24, 2026 04:55
The API executor marks transient failures (5xx, 408, 429, connection
errors) as retryable but never retried them. Add a spec.api.retries field
(default 0, preserving current behavior) that controls how many times a
transient failure is re-issued before the task fails, with a fixed 1s
backoff between attempts. Non-retryable 4xx statuses and cancelled tasks
are never retried.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
@timzsu
timzsu force-pushed the zsu/api-executor-retry branch from 04639c2 to 22b222e Compare September 27, 2026 01:13
timzsu and others added 5 commits September 27, 2026 11:16
…during backoff

A cancellation left over from a previous task no longer leaks into the next
task on a reused warm executor: cancel() records the task it targets, and
run() clears a cancellation addressed to a different task while one aimed at
the task now starting still stands, all under a single lock so a racing
cancel is never lost.

The retry backoff now waits on the cancel event instead of sleeping, so a
cancelled task raises its cancellation as soon as it is signalled rather than
sitting out the full backoff.

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>
Co-Authored-By: Claude Code <noreply@anthropic.com>
The interrupt monitor checks the runner's current task id and then calls
executor.cancel(task_id) without a lock spanning both, so a late
cancellation for a finished task can reach a warm executor that has since
started another task. The executor now records the active task id under
its cancel lock, sets the cancel event only when no run is in flight or
the cancellation matches the active task, and clears the active id when
the run returns or raises.

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>
Co-Authored-By: Claude Code <noreply@anthropic.com>
A single recorded cancellation id let a late cancel for a prior task
overwrite a recorded cancel for the task about to start, so that task
ran despite its own cancellation. Track pending cancelled ids in a set
guarded by the cancel lock; run() consumes only its own id and clears
the rest as stale.

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

@kaiitunnz kaiitunnz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. PTAL.

Comment thread src/worker/executors/api_executor.py
Comment thread src/worker/executors/api_executor.py
Comment thread src/worker/executors/api_executor.py
Comment thread tests/worker/test_api_executor.py Outdated
Comment thread tests/worker/test_api_executor.py
Comment thread src/worker/executors/api_executor.py
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
@timzsu
timzsu requested a review from kaiitunnz September 27, 2026 14:16

@kaiitunnz kaiitunnz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more minor issue.

Comment thread docs/WORKFLOWS.md Outdated
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>

@kaiitunnz kaiitunnz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@timzsu
timzsu merged commit 039aee4 into main Sep 27, 2026
11 checks passed
@timzsu
timzsu deleted the zsu/api-executor-retry branch September 27, 2026 14:49
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.

2 participants