Skip to content

GPU fault classification: cover the child pods of distributed tasks - #7913

Open
samhita-alla wants to merge 6 commits into
mainfrom
gpu-forward/child-pod-gpu-fault
Open

GPU fault classification: cover the child pods of distributed tasks#7913
samhita-alla wants to merge 6 commits into
mainfrom
gpu-forward/child-pod-gpu-fault

Conversation

@samhita-alla

@samhita-alla samhita-alla commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Why

#7879 classifies GPU faults into the failure the user sees, but only for tasks whose tracked resource is the Pod. CRD-backed distributed tasks — RayJob, PyTorchJob, MPIJob, TFJob, and the clustered JobSet plugin — track the CRD while the fault event lands on a worker pod the plugin manager never looks at, so an Xid on a training worker still surfaces as a generic operator failure. This closes that gap, which #7879's description tracks as its known follow-up.

How

Nothing new is watched. The executor's event cache already holds the Xid events recorded on operator-created pods, and every plugin already merges the framework's run, action and attempt labels into the pod templates it builds. What was missing is the lookup key from a failing CRD to its pods.

  • k8s.ChildPodDiscovery, an optional interface next to PluginAbortOverride and in the same shape: a plugin whose tracked resource is not a Pod can return the label selector that names the pods of this attempt. A plugin that does not implement it behaves exactly as before.
  • flytek8s.AttemptPodSelector builds the shared half of that selector from the labels the framework stamps on every task (managed, run, action, attempt), and returns nil when the attempt cannot be identified, because a partial match would credit one action's fault to another. The label keys move from the executor into flytek8s so the stamping side and the selecting side cannot disagree; flytek8s.PreservedPodLabels re-forces the same keys last in template builders so a user-supplied pod label named run or attempt cannot break the join.
  • Each plugin ANDs that with one operator-native label actually applied to its pods, so a sanitization collision cannot pull in another job's pods: Ray uses ray.io/cluster from the RayJob status (falling back to the attempt selector alone before the operator reports the cluster name), the three Kubeflow plugins share training.kubeflow.org/job-name, and clustered uses jobset.sigs.k8s.io/jobset-name. Every value is verified against the vendored operator, and a conformance test per plugin asserts the pod templates the plugin itself builds satisfy its own selector, so label drift on either side fails a unit test.
  • On a CRD task's failure, the plugin manager lists the child pods from the cache it already maintains (one List, no API call, no new informer, no RBAC change) and runs the same per-pod fault filter as the Pod path against each: reason, UID, and the relevance window. Pods that succeeded are skipped, so a benign mid-run fault on a worker that exited 0 cannot flip an unrelated user failure to a system one.
  • The relevance window anchors on each pod's own termination (latest terminated workload container, then deletion timestamp, clamped to the attempt's failure time) rather than the reconcile time the CRD plugins stamp, since for a CRD the phase timestamp says when the operator noticed, not when the pod died.
  • Cross-pod faults are ordered by when each fault was first created, so the earliest fault of the winning severity is the one named; the last observation is used only to decide whether a fault is still relevant. When classification settles on a child pod's fault, the failure carries one additional reason naming that pod, because on a many-worker job the Xid sentence alone does not say which worker to look at.

Faults on a child pod that was deleted before the failing reconcile are lost rather than guessed at; discovery logs at debug when that happens.

Tests

  • go build ./... in both modules; 38 packages tested across flyteplugins and the executor.
  • golangci-lint (v2.12.0) diffed against the base on both configs: no new findings.
  • New tests: AttemptPodSelector label cases; per-plugin ChildPods behavior including Ray's empty cluster name; per-plugin conformance of built pod templates against the plugin's own selector across every replica type; plugin-manager cases for earliest-critical-wins across workers, aggregated versus one-shot event ordering, succeeded workers excluded, still-running wedged workers included, different-attempt pods excluded, wrong UID rejected, vanished pods contributing nothing, user label overrides not breaking the selector, and non-implementing plugins unchanged.
  • The behavioral fixes were mutation-tested: reversing the sort order, removing the succeeded-pod skip, re-adding init containers to the anchor, removing the clamp, and reverting the label re-forcing each fail exactly the test written for it.

Stack: #7878 → #7879 (base) → this.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

This PR was flagged by our automated quality checks. If you're a genuine
contributor, please reply here and a maintainer will review your PR.

We appreciate your contribution and apologize if this is a false positive!

@samhita-alla
samhita-alla marked this pull request as draft August 25, 2026 13:11
@samhita-alla
samhita-alla force-pushed the gpu-forward/gpu-failure-classification branch from c56770a to 54e3a5e Compare August 26, 2026 06:43
This was referenced Aug 26, 2026
Merged
Merged
@samhita-alla
samhita-alla force-pushed the gpu-forward/gpu-failure-classification branch 2 times, most recently from 3176487 to f4abf56 Compare August 26, 2026 06:59
@samhita-alla
samhita-alla force-pushed the gpu-forward/child-pod-gpu-fault branch from 8ef1440 to e9767e0 Compare August 26, 2026 13:08
Base automatically changed from gpu-forward/gpu-failure-classification to main August 27, 2026 05:30
@samhita-alla
samhita-alla force-pushed the gpu-forward/child-pod-gpu-fault branch from e9767e0 to 23a0ece Compare August 27, 2026 06:05
samhita-alla and others added 2 commits September 15, 2026 15:37
@samhita-alla @claude
feat(executor): classify GPU faults recorded on a CRD task's child pods
1f4d2ac 
Classification so far covered tasks whose tracked resource is the Pod itself. A
RayJob, PyTorchJob, MPIJob or TFJob records the Xid on a worker pod while the
executor tracks the CRD, so the fault the node saw never reached the failure the
user read, and a GPU that fell off the bus during distributed training still
looked like the workload's own crash.

Nothing new has to be watched to fix that. The event informer is not scoped to
the tracked resource, so a worker pod's events are already cached, and the Pod
informer is already running and already selects on the managed label that every
plugin merges into the pod templates it builds. What was missing was a way to
name the pods: an operator's child pod names carry a random suffix, so the
executor cannot derive the keys the events are cached under.

ChildPodDiscovery is the optional interface that closes that, in the shape
PluginAbortOverride already established. A plugin returns the label selector that
picks out this attempt's pods; the manager lists them from the resource's own
namespace out of the cache it already holds, and reads each one's faults. Because
the pods arrive as objects, their UIDs are known, so the identity check that
already guarded the Pod path applies unchanged and never has to fall back to
matching on a name.

AttemptPodSelector builds the half of that selector which holds whichever
operator created the pod. The run, action and attempt labels move to flytek8s
next to the managed label, for the reason that one already lives there: the
plugins that build pod templates have to keep them intact. It refuses to build a
selector when any of them is missing, since run and action are dropped when
sanitization empties them and selecting on what is left would reach another
action's pods. PreservedPodLabels exposes the same key set for plugins to
re-apply after any labels a user supplied, so that the two halves cannot disagree
about which labels have to survive.

Which pods are consulted is narrower than everything the selector matches. A
worker that finished its work cannot be why the job failed, so a succeeded pod
contributes nothing even if it logged a fault mid-run; crediting that to a
launcher's user-code failure would turn a user error into a hardware one and stop
charging the retry. A pod still running is kept, because a worker wedged on a GPU
that fell off the bus is exactly the case this path exists for.

Relevance is anchored per pod on that pod's own container termination, because
every CRD plugin stamps the failure with the time of the reconcile that noticed
it rather than with anything the kubelet recorded, and anchoring on the reconcile
would age out a fault that killed a worker minutes earlier. Init containers are
not eligible anchors: they finish before the workload starts, and a native
sidecar declared among them is reaped after everything else. The anchor is never
later than the attempt's own failure, so a pod torn down long afterwards cannot
let faults recorded in the meantime explain it.

Which fault wins needs no new rule: ClassifyFailure already takes the worst
severity and the earliest within it, so the faults gathered across pods are only
sorted back into order before being handed over. That order is on when each fault
was first recorded rather than when it was last seen, because a fault that is
still repeating has a later last observation than a one-shot fault that followed
it, and ordering on that would let a downstream symptom outrank the root cause.
The last observation still decides relevance, which is all it was ever for.

The one pod whose fault settled the verdict is named as an additional reason,
because the fault sentence carries the Xid, the GPU and the node but not the pod,
which on a job with sixty-four workers is not enough to act on. Only that pod is
named: pointing at every pod that saw any fault would put a misleading reason on
a failure the faults did not explain, and would emit one cluster event per pod
when a whole node's worth of GPUs faults at once. The single pod path names
nothing, since the failure is already reported against that pod.

A gang-scheduled restart creates fresh pods under the same attempt and its faults
count, while an earlier Flyte attempt's pods are excluded by the attempt label. A
plugin that does not implement the interface contributes no pods and so no
faults, which is exactly what it did before.

A pod already torn down with its job is not recovered. The events are cached
under a name nothing can reconstruct, so discovery logs at debug and classifies
nothing rather than guessing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
@samhita-alla @claude
feat(ray,kfoperators,clustered): name the child pods of a distributed…
bd0682a 
… task

The Ray, Kubeflow and clustered plugins track a CRD, so they implement
ChildPodDiscovery to tell the framework where the pods that actually ran the task
are.

All three build the same selector: the attempt's own labels, which the pod
templates they hand the operator already carry, narrowed by one label the
operator applies to the pods themselves. For Kubeflow that is
training.kubeflow.org/job-name, which is the CR's own name and so is known from
the moment the resource exists; one implementation in common covers PyTorchJob,
MPIJob and TFJob, because all three get their replica pods from the same
operator. For the clustered plugin it is jobset.sigs.k8s.io/jobset-name, exported
by the vendored JobSet API the plugin already imports. For Ray it is
ray.io/cluster, which KubeRay stamps on head and worker pods and overwrites on
the template, and whose value is only knowable from the RayJob's status because
KubeRay appends a random suffix to the cluster name.

Until that status is reported Ray falls back to the attempt labels alone. That is
not a wider search in any sense that matters, since run, action and attempt
already pin the selector to one attempt of one action; the most it can add is the
job submitter pod, which has no GPU and so no faults to contribute. The
alternative, declining until the operator catches up, would lose the fault on
exactly the runs where the cluster never came up healthy.

Ray also has to stop a task from taking its own pods out of reach. The head and
worker templates let a task supply k8s_pod metadata, and that metadata was
unioned after the execution labels with only the managed label re-forced
afterwards, so a user label named run, action or attempt would overwrite the
identity the selector looks a pod up by. All three templates now re-apply
flytek8s.PreservedPodLabels last, which is the same key set the selector is built
from. Dask and Spark are left for a follow-up.

The conformance tests are the point of the test additions. Each asserts that the
labels the plugin puts on the pod templates satisfy the selector the same plugin
hands the framework, for every replica type it builds. The two halves live far
apart and drift on either one would leave a GPU fault on a worker silently
unclassified, which is the failure mode this whole path exists to prevent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
@samhita-alla
samhita-alla force-pushed the gpu-forward/child-pod-gpu-fault branch from 23a0ece to bd0682a Compare September 15, 2026 10:09
samhita-alla and others added 4 commits September 15, 2026 16:38
@samhita-alla @claude
fix(executor): stop excluding and mis-naming the pods a GPU fault cam…
49565fd 
…e from

Three things the executor decided about child pods were wrong.

The gate deciding whether to name the pod a fault was found on asked whether the
failure carried one of the gpufault codes, and it never opened on a real task.
ClassifyFailure only replaces a code the plugin had no opinion about, and the CRD
plugins all report codes of their own: ray reports TaskFailedWithError, kubeflow
DownstreamSystemError, clustered a JobSet condition reason. Those are exactly the
codes it is right to keep, so every distributed GPU failure lost the one line
saying which worker the hardware failed on. What actually says a fault was folded
in is the fault itself, which ClassifyFailure puts on the error in every case it
acts, so the gate reads that now and returns early only for a warning, which rides
along as data without deciding anything. Finding the pod still matches the settled
fault back by pointer, so it rests on ClassifyFailure passing the same fault
through rather than a copy; that invariant now has a test, and failing to find the
pod is logged rather than passed over, so breaking it cannot cost that line again
unnoticed.

Deciding a worker had finished by looking at all its containers called a pod
unfinished forever when a sidecar held it open, which is what a legacy-injected
service mesh proxy does: the work exits cleanly, the proxy never does, and a
faulting worker was excluded from classification for the life of the pod. What
settles it is the container running the task's own work, so that is what is asked
about now. Only a declared primary counts, through the new
DeclaredPrimaryContainerName, which reads the annotations and nothing else.
GetPrimaryContainerName cannot answer this: it falls back to guessing the first
container, and the container it picks may be the sidecar. Where nobody declared a
primary the older rule still stands.

The bound on a child pod's anchor was skipped when the plugin reported no failure
time of its own, on the reasoning that there was nothing to bound against. There
is: a pod the operator has not finished tearing down then anchored at the moment
of the reconcile and swept in every fault the node had recorded since. Now stands
in, which is what PodFailureTime already falls back to, so the two agree.

PreservedPodLabels also gains the task name. It takes no part in selection and is
deliberately absent from the selector's keys, but the per-pod metrics external
collectors scrape are joined on it, and a user label of the same name would point
those joins at the wrong task. The preserved set and the selection set are no
longer the same list, so each names its own keys and says why they differ.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
@samhita-alla @claude
fix(ray,clustered,kfoperators): select the right child pods, and only…
42ef918 
… those

Ray never stamped TaskInfo.OccurredAt, so the framework had no failure time for a
Ray task at all. Everything measured against it degraded: a GPU fault's relevance,
and the bound on a child pod's own anchor, which was skipped entirely for want of
anything to bound against. It now stamps the reconcile time the way every other
CRD plugin does.

Ray's fallback when the cluster name was not yet known selected on the attempt
labels alone, on the reasoning that those are exact to one attempt of one action.
They are not: they are sanitized and truncated to the 63 characters a label value
allows, so two long action names sharing a prefix collapse onto one value and
another action's pods would answer for this one. The fallback bought nothing to
weigh against that, because KubeRay fills the cluster name in before it creates the
RayCluster, so an empty one means no pod exists yet and there is nothing on record
to find. It now looks at nothing, and the comment says why instead of repeating the
claim that truncation breaks.

The clustered plugin selected every generation of a restarted JobSet at once. A
restart recreates every child pod under the same JobSet name, so name alone pooled
the pods of every generation the attempt had been through and let a fault from one
that died half an hour ago explain the failure of the one running now. The
selector now also carries the restart generation, which the JobSet controller
stamps on the pods it creates and which Status.Restarts holds for the generation
now running: exhausting the restart budget is what ends the attempt, and it does
not increment on the way out. While a restart is in flight the two disagree for an
instant and the selector matches nothing, which is the direction to fail in.

All three plugins now also name the container running the task's own work on the
pod templates they build, which is what lets the executor tell that work from a
sidecar that outlives it. Ray knows it for the head and the worker; the clustered
plugin already had it in hand for the JobSet; kubeflow's replica containers are
renamed to the operator's own fixed name by the webhook, which ToReplicaSpec has
just applied, so it is the same name for every replica role.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
@samhita-alla @claude
fix(executor): carry a failed attempt's reasons through an in-place r…
3186e4b 
…estart

An in-place restart replaces the failure with a Queued phase so the TaskAction
stays non-terminal. The replacement was built from nothing, so the reasons the
failed attempt had accumulated went with it, and on this path they are the only
account of what happened that ever reaches the user: the failure itself is
discarded and never reported. For a GPU fault that means the line naming which
worker the hardware failed on was written and then thrown away, leaving a task
that appears to have restarted for no reason.

queuedForInPlaceRestart carries the reasons across and nothing else. The rest of
the failed attempt's task info stays behind on purpose, because its logs and custom
info describe the pod that has just been deleted rather than the one being queued.
Naming the conversion also gives it somewhere to be tested, which the path it is
inlined in does not have without an API server.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
@samhita-alla @claude
fix(flytek8s): the framework's primary-container stamp outranks kubec…
ecc4bd9 
…tl's

DeclaredPrimaryContainerName read kubectl.kubernetes.io/default-container
ahead of the annotation the plugins stamp at build time. That let a pod
template pointing kubectl at a sidecar decide which container's exit
settles GPU fault classification: a sidecar exiting cleanly would mark
the pod succeeded while the real workload was still wedged, losing the
fault. Flyte's own stamp now wins; the kubectl annotation still answers
for pods the framework never stamped, where a declaration beats a guess.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
@samhita-alla
samhita-alla marked this pull request as ready for review September 15, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pingsutw pingsutw Awaiting requested review from pingsutw

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Code agent slop flyte2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@samhita-alla