Anchor spark-submit log trail on actual exception instead of lines - #70513
Conversation
shahar1
left a comment
shahar1
left a comment
There was a problem hiding this comment.
The anchor instinct is right — pinning on the JVM's own marker beats guessing at frame counts, and the K8s quota repro in the description is convincing. One thing I'd like to resolve before this lands, though: dropping the rolling fallback entirely means a whole class of spark-submit failures now comes back with no log tail at all.
No uncaught JVM exception → no output at all (spark_submit.py:892)
spark-submit exits non-zero without ever printing Exception in thread "..." in several common cases:
- A failing PySpark app.
SparkSubmit.doSubmitcatchesSparkUserAppExceptionand callsexitFn(e.exitCode)— the Python traceback goes to stdout, then a bare non-zero exit. No marker. - Argument / classpath errors —
Error: Failed to load class org.example.Main,Error: Missing application resource,Error: Master must either be yarn or start with spark, mesos, k8s.... These go throughprintErrorAndExit. No marker.
Before this PR those produced a 20-line tail containing the actual error text. After it, _submit_log_tail is "" and the user sees only Error code is: 1 — which is the problem #70178 set out to fix.
The description frames this as a deliberate trade, but I don't think the two behaviours actually conflict — keeping the rolling buffer as the fallback and letting the anchor reset it gets both (suggestion inline on spark_submit.py:892-896):
if not self._exception_anchor_seen and _EXCEPTION_START_RE.search(line):
# Drop the pre-exception banner noise, keep the whole trace from here on.
self._exception_anchor_seen = True
self._last_submit_log_lines = deque(maxlen=500)
self._last_submit_log_lines.append(line)…with __init__ back to deque(maxlen=20). Full trace when there is an exception, last-20 fallback when there isn't.
Smaller observations
See inline comments on spark_submit.py:61, spark_submit.py:327, and test_spark_submit.py:409.
This review was drafted by an AI-assisted tool and
confirmed by an Airflow maintainer. The findings
below are observations, not blockers; an Airflow
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.More on how Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.
Drafted-by: Claude Code (Opus 5); reviewed by @shahar1 before posting
|
All handled in 74da146 |
github-project-automation
Bot
moved this from In progress
to Done
in Durable / Crash-Safe Execution
3 participants
Was generative AI tooling used to co-author this PR?
Related to #70178
The previous fix kept a rolling tail of the last 20 lines of spark-submit's own output, but deep JVM stack traces (Fabric8/Vert.x/Netty chains routinely run 70+ lines through nested
Caused by:blocks) pushed the real exceptionmessage out of the buffer before the process finished, leaving only bare stack frames with no message text. A live K8s quota rejection test reproduced this exactly: the LLM retry policy saw only Netty I/O frames and misclassified
a permanent quota error as a transient SSL issue.
Anchor capture on the JVM's own uncaught-exception marker (
Exception in thread "...") instead of a blind line count. Everything before the marker is noise (banners, WARN logs) and is dropped; everything from the marker onward is kept, with a generous safety-valve cap rather than a tight rolling window. Ifspark-submitnever throws an uncaught exception,nothing is captured and the failure message carries no log tail, rather than a plain last-N tail of possibly-irrelevant output.
Testing
Error thrown now:
Earlier:
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.