[LIVY-1084] Prefer spark container exit status over pod phase when sidecars are enabled - #559
Open
soumyadeeplogin wants to merge 1 commit into
Conversation
…nabled A sidecar failure can mark the pod as "failed" even when the spark driver exited successfully. Remove the early return on non-running pod phase so that getTerminalState is always consulted first when sidecars are enabled, falling back to pod phase only when the spark container cannot be found. Add null safety for getContainerStatuses, which can be null while the pod is still in pending phase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
Filed LIVY-1084 for this change. |
Contributor
Author
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
1 participant
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.
What changes were proposed in this pull request?
When
livy.server.kubernetes.spark.sidecar.enabledistrue,SparkKubernetesApp.KubernetesAppReport.getDriverStatereturned the raw pod phase whenever the phase wasn't"running", before ever consulting the spark container's own termination status. A sidecar container that fails or gets OOMKilled after the spark driver container has already exited successfully can flip the pod phase to"Failed", which Livy then reports as the terminal application state even though the actual Spark job succeeded.This PR changes
getDriverStateso that, when sidecars are enabled, it always consultsgetTerminalState(the spark container's own exit code) first, and only falls back to the pod phase if the spark container cannot be found in the pod's container statuses. It also guardsgetTerminalStateagainstgetContainerStatuses()returningnull, which happens while the pod is still in thePendingphase, to avoid an NPE.Why are the changes needed?
To report the correct terminal application state (succeeded/failed) based on the actual Spark driver's exit code, rather than being misled by an unrelated sidecar container's failure flipping the pod phase.
Does this PR introduce any user-facing change?
Yes, but only when
livy.server.kubernetes.spark.sidecar.enabledistrue(the default)./sessions/:id/stateand/batches/:id/statewill now reportsucceededfor a pod whose spark container exited 0 even if a sidecar container's failure left the pod phase asFailed.How was this patch tested?
Added a unit test to
SparkKubernetesAppSpec(should prefer spark container exit status over pod phase when sidecars are enabled) covering: sidecar failure with a successfully-terminated spark container, a failed spark container, an indeterminate/not-yet-terminated spark container (falls back to phase), a pending pod with null container statuses (no NPE, falls back to phase), and sidecars disabled (phase is authoritative).Ran
mvn -pl server -am test -Dtest=SparkKubernetesAppSpec -DwildcardSuites=org.apache.livy.utils.SparkKubernetesAppSpec; all 12 tests pass.Was this patch authored or co-authored using generative AI tooling?
Yes, this patch was co-authored using Claude Code (Anthropic).