Skip to content

Detect Spark driver completion by container state when tracking via k8s API - #68048

Open
karenbraganz wants to merge 25 commits into
apache:mainfrom
karenbraganz:spark-container-status
Open

karenbraganz wants to merge 25 commits into
apache:mainfrom
karenbraganz:spark-container-status

Conversation

@karenbraganz

@karenbraganz karenbraganz commented Jun 5, 2026 •

Copy link
Copy Markdown
Collaborator

related: #67934

This PR tracks Spark job completion by container state instead of pod phase when track_driver_via_k8s_api=True. Sometimes the pod continues to run even after the driver container completes due to other sidecar containers. This PR makes driver completion detection more accurate by examining the container itself.

@karenbraganz

karenbraganz commented Jun 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

I still need to test this out and write unit tests.

@karenbraganz
karenbraganz requested a review from amoghrajesh June 9, 2026 16:39
@karenbraganz

Copy link
Copy Markdown
Collaborator Author

This has passed all unit tests as well as a manual test that I ran.

Comment thread providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py Outdated
@karenbraganz
karenbraganz requested a review from uranusjr July 21, 2026 13:48

@aaron-y-chen aaron-y-chen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also document the changes made in this PR in providers/apache/spark/docs/operators.rst?

Comment thread providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py Outdated

@aaron-y-chen aaron-y-chen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😃

Comment thread providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py Outdated
@karenbraganz
karenbraganz requested a review from eladkal September 9, 2026 19:25
@uranusjr

Copy link
Copy Markdown
Member

The original issue described a second problem

index 0 is not guaranteed to be the driver container in a multi-container pod.

This is reachable whenever driver_container is None and I think the PR does not address this? It’s fine to be fixed separately, but in that case the PR can’t close the issue outright.

@karenbraganz

Copy link
Copy Markdown
Collaborator Author

The original issue described a second problem

index 0 is not guaranteed to be the driver container in a multi-container pod.

This is reachable whenever driver_container is None and I think the PR does not address this? It’s fine to be fixed separately, but in that case the PR can’t close the issue outright.

I had initially decided to leave that part of the code as is because it only acts as a fallback when the driver container cannot be identified. However, now I am thinking it might be more accurate to just report the pod failure without the exact container status even in case of the fallback. Something like this:

if phase == "Succeeded":
    terminal_phase = phase
    break
if phase == "Failed" and not container_completed:
    raise RuntimeError(f"Spark application {app_id} failed (phase=Failed)")

Or we could print the statuses of all containers in the pod without assuming any specific one is the driver. @uranusjr what do you think?

I can create a separate PR for this. I have edited this PR description so that it dos not say that the issue is closed.

@karenbraganz
karenbraganz requested a review from uranusjr October 6, 2026 15:55

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants