manipatnam opened a new pull request, #74458:
URL: https://github.com/apache/airflow/pull/74458
When a task process is killed by a signal it did not raise itself, most
commonly by the kernel's OOM killer under a cgroup memory limit, the supervisor
survives and reports the task as `failed` on its behalf.
That report makes the task instance terminal within a few tens of
milliseconds, before the scheduler processes the executor event. The scheduler
then sees no discrepancy between the executor's view and the task record, so
`_process_executor_events` skips the state-mismatch branch, no
`TaskCallbackRequest` is built, and `handle_failure` is never called.
Two things follow from that:
* `on_failure_callback` does not run
* `on_task_instance_failed` does not fire, so OpenLineage emits no task FAIL
event for that try, and anything built on those events is blind to the failure
The supervisor cannot run the callback itself. It deliberately does not
parse the dag, and it has no database access. So instead of reporting a
terminal state it cannot act on, this leaves the task instance in `running` and
lets the scheduler finalise it through the path that already exists for tasks
whose whole pod died. That path builds the callback request and notifies
listeners.
#### Scope
Only tasks killed from outside change behaviour. Preserved exactly as before:
* a kill this supervisor initiated: graceful shutdown, heartbeat failure,
task overtime, server-requested termination, startup abort
* an ordinary non-zero exit code
* any state the task reported before dying, which always wins
* `SERVER_TERMINATED`, which returns before the new branch
Tasks with retries remaining are also unaffected. That state is already not
sent directly, and the resulting callback is correctly typed `UP_FOR_RETRY`, so
`on_retry_callback` is the hook that runs. A retrying task is not failing, so
`on_failure_callback` firing there would be wrong.
#### Observed behaviour this fixes
On Airflow 3.3 with the KubernetesExecutor, under a cgroup v2 memory limit
with `memory.oom.group=0` (the Kubernetes default), the kernel kills only the
largest process in the container. That is the task, not the supervisor. So the
supervisor-survives case is the common one, and it is the only one that is
broken.
In a controlled comparison of three tasks that OOMed identically against the
same 600 MiB limit, the only variable that decided whether a callback ran was
whether retries remained:
retries exhausted -> supervisor reports failed in ~30 ms -> no
callback, no FAIL event
retries remaining -> supervisor reports nothing -> scheduler
finalises, callback runs
#### Trade-off
A signal-killed task is now finalised by the heartbeat check rather than
immediately, so it stays `running` for up to `[scheduler]
task_instance_heartbeat_timeout` (default 300s) and holds its pool slot for
that window. Deployments that need this shorter can lower that setting together
with `task_instance_heartbeat_timeout_detection_interval`.
This is a user-facing behaviour change and will need a newsfragment.
#### Tests
Supervisor side, `task-sdk/tests/.../test_supervisor.py`:
* SIGKILL and SIGSEGV are both deferred to the scheduler
* a supervisor-initiated kill still reports
* a state the task reported itself still wins
* a non-signal exit code is unaffected
* `kill()` records that the signal came from us
Scheduler side, `airflow-core/tests/unit/jobs/test_scheduler_job.py`:
* a task left `running` with a stale heartbeat and no retries produces a
`TaskCallbackRequest` typed `FAILED` and fires `on_task_instance_failed`
* the same task with retries remaining produces one typed `UP_FOR_RETRY`
#### Open questions
1. Is the latency acceptable, or should the supervisor instead report the
terminal state *and* signal the server to enqueue the callback? That is the
approach proposed in #60858 (comment 6052760494) and removes the delay
entirely, at the cost of an execution API change. I am happy to switch to it if
that is preferred; this PR is the smaller option that needs no API change.
related: #60858
--
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]
