GPU fault classification: typed failure codes from Xid events - #7879
Conversation
ce23533 to
ed6079e
Compare
ed6079e to
f30439b
Compare
f30439b to
5a807c3
Compare
6f97cfb to
d2c6afb
Compare
d2c6afb to
80a5f38
Compare
a7a919f to
cb92c00
Compare
cb92c00 to
c56770a
Compare
c56770a to
54e3a5e
Compare
A node-level agent can turn NVIDIA Xid and NVSwitch SXid lines from the kernel log into a Kubernetes Warning Event on the task pod. Until now the message format, the Xid name and severity tables and the parser had no home in this repo, so an emitter and a consumer could only agree on them by copying, and drift. This adds flyteplugins/go/tasks/pluginmachinery/gpufault as the one definition: the event message format (FormatEventMessage, ParseEventMessage, and Sentence for the human half on its own), the Xid tables behind NameFor and SeverityFor, and the conversions to and from the typed core.GpuFault, including FromEventMessage which turns an arbitrary pod event into a fault or into nothing. The wire format is unchanged, byte for byte, because events already recorded on running clusters have to keep parsing. On top of that it adds the classification the executor applies to a failed attempt. CodeFor names the failure (GpuFallenOffBus, GpuEccUncorrectable, GpuRowRemapPending, GpuNvlinkError, GpuGspError, GpuXidError) and ClassifyFailure folds the faults observed on the pod into the failure the plugin reported: a critical fault makes it a system retryable failure so it does not burn a user retry on hardware the workload did not break, a user fault only names what went wrong and leaves the plugin's verdict alone, and a warning rides along as data. Every rule keeps the error URI, timestamp, worker, recoverability, task info and phase version the original failure carried. The package has no Kubernetes dependencies so both the emitter and the consumers can import it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
…error DemystifyFailure worked out a failure code from the pod status reason and the container states and then threw it away on the system path, reporting every system error as Interrupted. A pod killed by a graceful node shutdown reached the user as Interrupted whether the kubelet had said Shutdown, NodeShutdown, Terminated or NodeAffinity, and that reason is the only record of what happened to the node. The system path now reports the code it worked out. The two places that mean Interrupted still say Interrupted: the SIGKILL branch sets it deliberately, and the branch where the kubelet recorded nothing before the node went away now sets it explicitly instead of relying on the return to overwrite UnknownError. Nothing outside this function compares a failure code to Interrupted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Two things had to change for a GPU fault to reach the user. The Kubernetes plugin manager now looks for GPU faults when an attempt ends in a failure on a pod. It reads every event recorded against that pod over the whole attempt rather than only the ones since the event watermark, because the Xid that killed the task is usually recorded rounds before the pod's status catches up with it, turns each one into a core.GpuFault, and hands the list to gpufault.ClassifyFailure. Nothing else in the executor sets ExecutionError.gpu_fault. toActionErrorInfo then carried only the message and the kind onto the action event, so the code the plugin had worked out was dropped on the floor: a task killed for running out of memory reached the console and the SDK with no code at all. It now carries the code and the GPU fault through as well. The CR-persisted ErrorState already round-tripped the code, so that path needed nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Four defects from code review of the classification path. Events are now trusted by their reason, not their text: the executor only parses events whose reason is the emitter's GPUXidError or GPUSXidError (constants the gpufault package now exports), and FromEventMessage re-derives severity from the local table instead of honoring the message tail, so free text in an event cannot pose as a critical fault. The event search is bounded by a ten-minute relevance window, so an old fault on a long-lived pod cannot reclassify every later, unrelated failure. ClassifyFailure keeps the failure's shape: a permanent failure stays permanent (the fault reclassifies whose problem it is, not whether the task can run), a specific plugin code such as OOMKilled is kept and only generic codes give way to the fault's, cleanupOnFailure survives the rebuild (a new WithCleanupOnFailure helper; WithVersion also no longer drops the flag), and Recoverability is no longer copied across a verdict change. The severity table now agrees with the code table: 140 (unrecovered ECC), 62 and 109 are critical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Review follow-up on the classification scope. Identity now comes from the event's regarding UID matched against the pod being classified, so a recreated pod with a reused name cannot inherit its predecessor's faults; the name-plus-window heuristic no longer carries that job. Recency comes from the event's last observation: the watcher no longer ignores updates, which is how Kubernetes delivers an aggregated recurring event, and stores the freshest of eventTime, series.lastObservedTime and lastTimestamp without touching the created/recorded watermarks other consumers advance on. The relevance window now bounds only how long a quiescent fault stays relevant to new failures on the same pod, evaluated against that last observation, so a fault still recurring classifies while one that stopped long ago does not. Refreshed entries are stored as new values rather than mutated in place; list hands the pointers to readers outside the lock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
…se it The emitter reads context the code table cannot see, such as the driver labelling an NVSwitch SXid non-fatal. Trusting a downgrade is safe (making a fault less alarming gains an attacker nothing that silence would not) while an upgrade stays blocked, so classification keeps deciding retry budgets from its own table at worst. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Interrupted, the verdict DemystifyFailure reaches for a pod killed or lost without the kubelet recording why, now counts as a generic code, so a recorded critical fault names the failure (GpuFallenOffBus and friends) instead of being refused by the keep-specific-codes rule; that was the flagship hardware-fault path and it produced only Interrupted. Event identity no longer has a bypass: an event without a regarding UID is rejected outright, since the API server never fills that field and its absence means the client did not say which object it meant. When the pod itself is unknown (deleted before the round reached it) name-keyed events are accepted rather than discarding every fault on the path where hardware most clearly failed. ParseEventMessage locates the contract prefix instead of requiring it at offset zero, because the recorder's aggregator rewrites a note to "(combined from similar events): ..." once enough distinct messages share a key, which is exactly the storm case. An unrecognized severity label is reported as unknown and falls back to the table rather than silently parsing as warn and slipping under the downgrade-only clamp. The relevance window is thirty minutes, long enough to span node-NotReady grace plus pod eviction, and faults skipped as stale are logged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
…urately The comment still said ten minutes and explained only the status-catch-up gap; it now states the two slow paths the thirty-minute window is sized for, and the function's own doc comment sits on the function rather than on the constant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
… now The relevance window compared a fault's last observation against the classification time, which answers "was this fault recent when we looked" rather than "did this fault precede this failure closely enough". A slow reconcile could age a real cause out, and a fault observed after the failure could be credited to it. The window is now measured from the failure's own time, taken from the pod plugin's TaskInfo.OccurredAt (the container termination the kubelet stamped on the same node as the fault events), with a short slack for a fault recorded moments after the failure it caused and nothing beyond that; a future-dated event can no longer stay relevant indefinitely. The classification time is only the fallback when a plugin stamped no occurrence time. The identity comments are tightened to match the code: an event without a regarding UID is rejected outright, and the name-only match when the pod's own UID is unknown is stated as the deliberate trade it is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
54e3a5e to
3176487
Compare
…r's error When the plugin reported a failure with no reason of its own, a generic code such as Interrupted or a bare exit status, and a user-class fault such as Xid 31 was recorded on the pod, the classification kept the error kind the plugin guessed. For a pod that was killed that guess is SYSTEM, so a kernel that faults its own GPU was replayed against the system retry budget, up to thirty attempts that each break another CUDA context to reach the same answer. The user branch now sets the kind to USER alongside the GpuXidError code whenever it replaces a generic code, so the failure spends the task's own retries. A plugin verdict that carried a specific reason (OOMKilled, NodeShutdown) keeps both its code and its kind, since the fault may be incidental to it. Retryable versus permanent remains the plugin's call either way. Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
3176487 to
f4abf56
Compare
| observedAt := event.LastObservedAt | ||
| if observedAt.IsZero() { | ||
| observedAt = event.CreatedAt | ||
| } |
There was a problem hiding this comment.
should we do?
firstSeen := event.CreatedAt
if firstSeen.IsZero() { firstSeen = event.LastObservedAt }
if lead := failureAt.Sub(firstSeen); lead > window || lead < -slack { skip }because Fault starts 1 min before the container dies, keeps
firing for 11 min after → lead ≈ -11m < -2m → dropped. The louder and longer the hardware screams, the more certain the fault is discarded.
There was a problem hiding this comment.
now i'm treating the event as active over [CreatedAt, LastObservedAt] and keeping it when that interval overlaps [failureAt - 30m, failureAt + 2m] instead of just using CreatedAt because a fault first recorded 40 minutes before the failure (outside the 30-minute window) can still be firing at failure time.
| // The failure's own time anchors relevance. The pod plugin stamps it from the | ||
| // container's termination, which the kubelet recorded on the same node and clock | ||
| // as the fault events; when a plugin did not, the classification time stands in. | ||
| failureAt := time.Now() | ||
| if info := phaseInfo.Info(); info != nil && info.OccurredAt != nil && !info.OccurredAt.IsZero() { | ||
| failureAt = *info.OccurredAt | ||
| } |
There was a problem hiding this comment.
The pod plugin stamps it from the // container's termination
Should we check if phaseInfo is terminated or not? otherwise, the failureAt may become container start time
There was a problem hiding this comment.
terminal check is already there. classifyGpuFailure returns early unless phaseInfo.Phase().IsFailure(), so we never get here on a non-terminal phase. but OccurredAt comes from GetLastTransitionOccurredAt which returns a running container's StartedAt when nothing terminated, so a pod evicted while running anchored the window on a start time hours in the past and discarded every real fault.
the anchor is now derived from the pod itself: latest terminated main-container FinishedAt, then DeletionTimestamp.
The Xid name and severity tables were transcribed from NVIDIA's documentation but did not say so, so a reader had no way to check an entry against its source or to tell whether a missing code was an oversight or deliberate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
A fault that keeps repeating is aggregated by Kubernetes into a single event whose last observation moves with every repeat, so an event describes an interval and not a moment: first recorded at one time, still firing at another. Relevance was decided on the last observation alone, which discarded the case that matters most. Hardware that goes on faulting after the container died has a last observation well past the failure, so a fault created a minute before the pod died but still firing eleven minutes later was rejected for being two minutes too late, and the longer the hardware kept faulting the more certainly its fault was thrown away. Deciding on the creation alone would break the opposite case, which the previous behaviour got right: a fault first recorded before the window opened but still firing when the task died is exactly the fault that killed it. faultOverlapsFailure keeps both. The event is active over the stretch from when it was first recorded to when it was last seen, the failure is relevant over the window before it and the small slack after it, and the fault counts when those two stretches overlap. A fault that only started after the failure is still rejected, and so is one that had stopped firing before the window opened. The slack's meaning is narrowed to match: it bounds when a fault started, not when it stopped, because how long dying hardware goes on faulting says nothing about whether it caused the failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
…t time The failure's own time is what the relevance interval is centred on, and it came straight from the phase info the plugin reported. That time is GetLastTransitionOccurredAt, which for a pod that failed while its containers were still running is the time the running container started. A task evicted after six hours therefore anchored on a moment six hours before anything went wrong, and every fault the node actually recorded fell outside the window and was thrown away. podFailureTime derives the anchor from the pod instead. The kubelet stamps a container's termination on the same node and clock as the fault events, so the latest terminated container is the closest thing to the moment a fault would have to explain. A pod on its way out without a terminated container is anchored on its deletion, which is what an eviction leaves behind. Only then does the plugin's reported time stand in, and the classification time after that. Init containers are not eligible anchors. They finish before the workload starts, and a native sidecar declared among them is reaped after everything else, so either would anchor on a moment that has nothing to do with when the work died. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Samhita Alla <aallasamhita@gmail.com>
Why
Two things stand between a GPU fault on a node and the user who has to act on it.
The first is that failure codes get dropped on the way out.
flytek8s.DemystifyFailureworks out a code from the pod status and the container states and then throws it away on the system path, reporting every system error asInterrupted, so a graceful node shutdown reaches the user with no trace ofShutdownorNodeShutdown.toActionErrorInfoin the executor then builds aworkflow.ErrorInfoout of the message and the kind only, so evenOOMKillednever makes it onto the action event.The second is that nothing classifies GPU faults at all. Where a node-level agent records NVIDIA Xid and NVSwitch SXid faults as Warning Events on the task pod (the message format is defined in the new
gpufaultpackage), the executor already forwards pod events into the attempt's cluster events, but the failure the user sees is still whatever exit code the container produced. A GPU that fell off the bus and a program with a bug look the same. A hardware fault is the platform's problem, not the user's, and the failure should say so instead of burning a user retry.Known limitation, tracked as a follow-up: classification covers tasks whose tracked resource is the Pod itself. CRD-backed tasks (RayJob, PyTorchJob, MPIJob) record the fault event on a worker pod while the executor tracks the CRD, so they are not classified yet; covering them needs per-plugin child-pod discovery in the event lookup, implemented in #7913 (stacked on this PR).
What
New shared package
flyteplugins/go/tasks/pluginmachinery/gpufault. It holds the event-message contract (format, parser, Xid name and severity tables) so that emitters and consumers share one definition, the conversions to and fromcore.GpuFault, and the classification rules. The wire format is unchanged byte for byte, because events already recorded on running clusters have to keep parsing.ClassifyFailure(phaseInfo, faults)folds the faults observed on a failed attempt's pod into the failure the plugin reported:gpu_faultCodeFor(fault), e.g.GpuFallenOffBusOOMKilled,NodeShutdown)UnknownError,Interrupted,Error, empty, a bare exit status)GpuXidErrorA critical Xid means the device or the node is not trustworthy and the workload did not cause it, so it must not consume a user retry, and the reschedule wants to land elsewhere once phase 3 quarantines the node. A user Xid is the workload's own doing (NVIDIA documents 13, 31, 43 and 45 as application-caused: illegal address, out-of-bounds access, illegal instruction, a dead context preempted). The rule for it: if the plugin had a reason of its own for the failure, that verdict stands in full and the fault only adds the driver's sentence to the message. If all the plugin could say was that the pod died (
Interruptedis the kubelet-left-no-reason guess, and it guesses SYSTEM), the recorded fault is the explanation, so the code becomesGpuXidErrorand the kind becomes USER: a kernel that faults its own GPU will fault again, and replaying it against the system retry budget burns up to thirty attempts, each breaking another CUDA context, to reach the same answer. Retryable versus permanent stays the plugin's call either way, so a permanent failure is never downgraded to retryable. The cost of this rule being wrong is benign: the rare Xid 31 that is really a bad device shows up as a user error with the exact Xid, GPU and driver sentence on it, and the user reruns; the cost of the opposite rule is the retry storm. The error URI, timestamp, worker, recoverability, task info and phase version of the original failure are preserved in every case.DemystifyFailurereports the code it worked out on the system path instead of flattening everything toInterrupted. The SIGKILL branch and the branch where the kubelet recorded nothing before the node went away still reportInterrupted, now explicitly.toActionErrorInfocarriesCodeandGpuFaultontoworkflow.ErrorInfo. The CR-persistedErrorStatealready round-trips the code, so that path needed no change.Executor wiring:
PluginManager.classifyGpuFailureruns afterattachRecentObjectEventswhen the attempt failed and the resource is a Pod. It lists every event on the pod for the whole attempt rather than only since the event watermark, because the Xid is usually recorded rounds before the pod status catches up with it. Nothing else in the executor setsExecutionError.gpu_fault.Behaviour change
UnknownError/Interrupted, user retryable, message is the pod statusGpuFallenOffBus, system retryable, message starts with[gpu-health] [CRITICAL] Xid 79 (GPU has fallen off the bus) on GPU 0 GPU-…,gpu_faultsetUnknownErrorGpuXidError, message starts with[gpu-health] [USER] Xid 31 (GPU memory page fault) …,gpu_faultsetInterrupted, retried against the system budgetGpuXidError, same message shape, fails once unless the task has retriesOOMKilledgpu_faultsetOOMKilledgpu_faultfor the consoleNodeShutdownInterruptedNodeShutdownTests
go build ./...,go vet ./flyteplugins/... ./executor/...go test ./flyteplugins/go/tasks/pluginmachinery/... ./flyteplugins/go/tasks/plugins/k8s/pod/... ./executor/pkg/...golangci-lint run --tests=false(v2.12.2, the pinned version) inflyteplugins/andexecutor/on the touched packages: no new findings.DemystifyFailuresystem codes,toActionErrorInfo, andclassifyGpuFailureagainst a fake event watcher.Stack: #7878 (base) → this.
🤖 Generated with Claude Code
main