1fanwang opened a new pull request, #66405:
URL: https://github.com/apache/airflow/pull/66405

   Concrete draft behind **[AIP-97: Disruption 
Readiness](https://cwiki.apache.org/confluence/x/0ITMFw)** (DRAFT on cwiki), 
opened so the listener API can be reviewed against working code.
   
   ## Why
   
   
[`on_task_instance_failed`](https://github.com/apache/airflow/blob/0de4133d8d/shared/listeners/src/airflow_shared/listeners/spec/taskinstance.py#L52)
 only sees the worker-side `error`. A task killed from outside (eviction, OOM, 
force-delete, lost heartbeat) never raises, so an eviction and a real 
`ValueError` read the same to alerting, lineage, and the retry counter. Airflow 
already requeues a pre-start pod without spending a retry; this carries that to 
a running task.
   
   ## What
   
   **Pillar 1.** An optional `failure_kind` enum (`infra`, `application`, 
`timeout`, `manual`) and a short `reason` token on the 
[hook](https://github.com/apache/airflow/blob/0de4133d8d/shared/listeners/src/airflow_shared/listeners/spec/taskinstance.py#L56);
 [pluggy](https://pluggy.readthedocs.io/en/stable/) matches by name, so a 
listener that omits them is unchanged. The Kubernetes executor's 
[`classify_pod_failure()`](https://github.com/apache/airflow/blob/0de4133d8d/providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py#L300)
 reads pod details it already collects.
   
   **Pillar 2 (opt-in).** On `infra`, refund the attempt instead of charging 
`retries` (reuses `max_tries`, capped by `max_infra_refunds`), off by default.
   
   ## No database change
   
   The reason reaches the listener as a transient argument, not a column, so 
AIP-97 needs no migration and backports cleanly with no upgrade or rollback 
step. An earlier revision carried an `infra_reason` column; it is dropped. 
Whether the token rides this `reason` argument or folds into `error` is open 
question 3 ([#23](https://github.com/1fanwang/airflow/pull/23) vs 
[#22](https://github.com/1fanwang/airflow/pull/22)). The diff reads as three 
parts, shown split in 
[#11](https://github.com/1fanwang/airflow/pull/11)/[#12](https://github.com/1fanwang/airflow/pull/12)/[#13](https://github.com/1fanwang/airflow/pull/13)
 (the column-era cut).
   
   ## Related POCs
   
   [Celery](https://github.com/1fanwang/airflow/pull/15), 
[heartbeat/LocalExecutor](https://github.com/1fanwang/airflow/pull/14), 
[callbacks](https://github.com/1fanwang/airflow/pull/19), 
[metrics](https://github.com/1fanwang/airflow/pull/20) ([live 
OTel/Prometheus/Grafana 
e2e](https://github.com/1fanwang/airflow/tree/aip97-metrics-evidence/dev/aip97-metrics-evidence)),
 [logs](https://github.com/1fanwang/airflow/pull/21), 
[docs](https://github.com/1fanwang/airflow/pull/18), open questions 
[#16](https://github.com/1fanwang/airflow/pull/16)/[#17](https://github.com/1fanwang/airflow/pull/17).
   
   ## Testing
   
   Core unit suite on this no-DB tree, all green (33):
   
   ```
   test_taskinstance_infra_refund.py ........          8 passed
   test_taskinstance_failure_kind_sources.py .......   7 passed
   test_listener_types.py ..                           2 passed
   cncf/kubernetes/.../test_classify_pod_failure.py    16 passed
   ```
   
   Live on a real DB with a registered listener 
([#23](https://github.com/1fanwang/airflow/pull/23)): the listener gets 
`failure_kind=INFRA` and `reason='Evicted'` as transient args, `max_tries` goes 
2 to 3, and no `infra_reason` column exists. Migration head map unchanged, so 
`airflow db migrate` is a no-op. Both providers still import on Airflow 3.3.0 
(their floor); `classify_pod_failure` is gated to 3.4+.
   
   ## CI note
   
   `K8S Lang-SDK` is green after rebasing onto `main`, which carries the 
lang-SDK `go.mod` fix ([#70561](https://github.com/apache/airflow/pull/70561)) 
behind the original red. The remaining provider-suite reds are all one flaky 
test, 
`providers/ssh/.../TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control`
 (1 failed / ~19k passed per job). It reds `main` itself and is not 
deterministically reproducible, per 
[#70562](https://github.com/apache/airflow/pull/70562) from a PMC member 
("failed on main", "I could not reproduce", clears on rerun). This diff does 
not touch `providers/ssh`; every AIP-97 and `cncf.kubernetes` test passes.
   


-- 
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]

Reply via email to