Skip to content

fix: clear stale error.pb at the start of every clustered attempt - #1519

Merged
AdilFayyaz merged 1 commit into
mainfrom
adil/jobsets-stale-errorpb-cleanup
Sep 1, 2026
Merged

fix: clear stale error.pb at the start of every clustered attempt#1519
AdilFayyaz merged 1 commit into
mainfrom
adil/jobsets-stale-errorpb-cleanup

Conversation

@AdilFayyaz

Copy link
Copy Markdown
Collaborator

Motivation

For clustered (JobSet) tasks the SDK writes error.pb only on what it believes is the terminal
restart attempt, using JOBSET_RESTART_ATTEMPT >= JOBSET_MAX_RESTARTS. That guess mirrors the
JobSet's Status.Restarts, which also counts free host-maintenance restarts
(RestartJobSetAndIgnoreMaxRestarts, flyteorg/flyte#7947), while the budget is charged from
RestartsCountTowardsMax, which pods never see. So after a free restart the guess fires early, a
genuine failure writes error.pb on a non-terminal attempt, and because the executor reads
error.pb before outputs.pb, a later successful attempt is still reported as a failure.

Summary

  • Add io.clear_stale_clustered_error(), which removes a leftover error.pb from the attempt's
    output prefix. It runs on every clustered rank-0 start, not just restarted ones, since the
    restart counter is exactly what proved untrustworthy. Only rank-0 deletes, restart attempts are
    strictly sequential, and a failed delete only restores today's behavior.
  • Await it at the top of load_and_run_task, ahead of the code-bundle download and of all three
    upload_error call sites.
  • Keep the terminal-attempt gate as an optimization that avoids needless writes, and rewrite its
    docstring: the guess can be early but never late, and the cleanup is what guarantees correctness.
  • Rewrite examples/clustered/ddp_train_restart.py into a regression test that reproduces the
    stale file with no backend dependency, using max_restarts=0 plus one in-pod torchrun restart.

Test Plan

  • uv run pytest tests/flyte/clustered tests/flyte/internal/runtime tests/flyte/internal/bin -q
    covers the async and sync delete branches, absent-file and non-rank-0 and non-clustered no-ops,
    soft failure on both the existence check and the delete, and that the cleanup is awaited before
    the task load, the load-failure error upload, and the task body. 364 passed.
  • make lint and make mypy are clean.
  • End to end on dogfood: make dist && uv run python examples/clustered/ddp_train_restart.py.
    Expect SUCCEEDED. flyte get logs <run> should show attempt 0 uploading .../a0/0/error.pb,
    then the restarted rank-0 logging Removed stale ... error.pb (JOBSET_RESTART_ATTEMPT=0, TORCHELASTIC_RESTART_COUNT=1), then training. Verified as run udvvrchxvfqpnwjvbfwp.
    Reverting the entrypoints.py hook makes the same run end FAILED with the attempt-0 error.

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
@AdilFayyaz AdilFayyaz self-assigned this Sep 1, 2026
@AdilFayyaz
AdilFayyaz requested a review from pingsutw September 1, 2026 22:02
@AdilFayyaz
AdilFayyaz merged commit c872581 into main Sep 1, 2026
58 checks passed
@AdilFayyaz
AdilFayyaz deleted the adil/jobsets-stale-errorpb-cleanup branch September 1, 2026 22:21
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