kfpytorch: fail recoverably when the elastic agent is signalled instead of raising IgnoreOutputs - #60
Open
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…ad of raising IgnoreOutputs A SIGTERM to the elastic agent (spot/preemption, eviction, operator teardown) surfaced as IgnoreOutputs, which Flyte records as a succeeded attempt without outputs: retries and auto-resume never fire and a preempted job dies green. Re-raise it as FlyteRecoverableException (cause + timestamp preserved) so the attempt fails and is retried. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracking issue
Related to upstream flyteorg#2064 (the change that introduced
SignalException -> IgnoreOutputs). Exa incident: GS1 execazvg5f9v9974m78fvlf7(BIGMODEL, 2026-09-10).Why are the changes needed?
PytorchElasticFunctionTask._executecatches torch elastic'sSignalException(raised by the agent's SIGTERM/SIGINT handler) and turns it intoIgnoreOutputs.flytekit/bin/entrypoint.pytreatsIgnoreOutputsas a clean exit: noerror.pb, nooutputs.pb, process exits 0, so propeller marks the attempt succeeded.On a spot / low-priority preemption the kubelet SIGTERMs the pod while training is still running. Result: the execution ends
SUCCEEDEDwith no outputs and no checkpoint,retriesandauto_resumenever fire, and the job dies silently green. Whether a preempted job retries or "succeeds" is a race on which pod gets the signal first.Evidence (read-only, Loki + Overseer):
azvg5f9v9974m78fvlf7—19:45:22Z Received 15 death signal, shutting down workers→19:45:29Z Plugin [container] returned no outputReader ... PhaseSuccess→ executionsucceeded,s3://exa-models/checkpoints/gs1-distill/bigmodel-c42-nem1b-a2-lr5em5-v1/empty.The
IgnoreOutputspath came from upstream (flyteorg#2064, to keep the operator's SIGTERM of sibling pods after another worker's failure from masking that failure). This is not what it does in practice: withEarliestErrorAggregationStrategy(hardcoded for the pytorch plugin in flyteplugins) the sibling's own, earlier error already wins on timestamp; swallowing the signal only hides genuine preemption.What changes were proposed in this pull request?
except SignalException as e: logger.exception(f"Elastic launch agent process terminating: {e}") - raise IgnoreOutputs() + raise FlyteRecoverableException( + f"Elastic launch agent process was terminated by signal {e.sigval.name} before the worker " + f"group finished: {e}", + timestamp=time.time(), + ) from eFlyteRecoverableException(USER:Recoverable) is what the plugin already raises for recoverable worker errors, so propeller counts the attempt againstretriesand relaunches;auto_resumethen finds the last checkpoint.timestampis carried into theContainerErrorwritten by the entrypoint, so earliest-error aggregation still attributes a task failure to a sibling worker whose error predates the teardown signal.Raises:updated.IgnoreOutputsis still raised for worker groups with index > 0 on success (unchanged).How was this patch tested?
Two tests added to
plugins/flytekit-kf-pytorch/tests/test_elastic_task.py:test_agent_signal_is_recoverable_failure— deterministic:elastic_launchpatched to raiseSignalException(SIGTERM); assertsFlyteRecoverableException(notIgnoreOutputs),__cause__is theSignalException, message namesSIGTERM, timestamp set.test_sigterm_to_agent_process_is_recoverable_failure[spawn|fork]— end to end: realElastic(nnodes=1, nproc_per_node=2), rank 0os.kill(os.getppid(), SIGTERM)s the agent after a gloo barrier (the barrier guarantees every worker is forked so the signal lands in the agent's monitor loop, not insideos.fork()where CPython's fork warning machinery drops the handler's exception). Both start methods raiseFlyteRecoverableException. On the base branch the same test observesIgnoreOutputs.Setup process
uv venv && uv pip install -e . -e plugins/flytekit-kf-pytorch[elastic] pytestScreenshots
n/a
Check all the applicable boxes
test_end_to_endabove).Related PRs
Upstream origin of the swallowed signal: flyteorg#2064. Monorepo pin bump to follow once merged (
flytekitplugins-kfpytorch[elastic] @ .../exa-labs/flytekit/<sha>.tar.gzinpython/shared/exa_flyte/pyproject.tomland friends).Docs link
n/a
Link to Devin session: https://app.devin.ai/sessions/84936c9760074a4793d713c917952f29
Open in Devin Desktop: https://app.devin.ai/desktop/session/84936c9760074a4793d713c917952f29?variant=devin
Requested by: @jld-adriano