seanmuth commented on PR #73142: URL: https://github.com/apache/airflow/pull/73142#issuecomment-5683509433
Correction to my own earlier framing: the "an unobserved/defaulted exit code overriding an already-confirmed terminal state" explanation in the current docstring is inference I hadn't actually verified. I built live instrumentation and reproduced the crash under load — full writeup with tracebacks and the apiserver-side timeline is in https://github.com/apache/airflow/issues/65708#issuecomment-5683506198. Short version: the real, reachable case is `update_task_state_if_needed()` computing `final_state=FAILED` from a genuinely non-zero `exit_code` despite `_terminal_state` already being `SUCCESS`, then calling `.finish()`, which 409s against the row a prior `.succeed()` call already wrote correctly — and that exception is uncaught in `wait()`/`supervise()`. Traced end to end against this PR's fix: with correct precedence, `final_state` resolves to `SUCCESS`, lands in `STATES_SENT_DIRECTLY`, and `.finish()` is never called. This PR's change holds up against the live-captured case; the docstring's cited scenario (`_handle_process_overtime_if_needed()`/SIGTERM) was just the wrong example, not a wrong fix. Holding off on any further changes here pending review. --- Drafted-by: Claude Code (Sonnet 5); reviewed by @seanmuth before posting -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
