aunderwood24 opened a new pull request, #72062:
URL: https://github.com/apache/airflow/pull/72062

   closes: #72061
   related: #68243, #71540
   
   ## Why
   
   On MySQL, `Trigger.clean_unused()` deletes unreferenced triggers in two 
steps: SELECT the candidate ids into a Python list, then DELETE by id. A task 
that defers between those two statements attaches to a trigger that's already 
on the list; the DELETE fires anyway, and the `ON DELETE CASCADE` on 
`task_instance.trigger_id` silently deletes the task instance row. The dag run 
then either fails via "Task deadlock (no runnable tasks)" with zero failed 
tasks (so no failure callback ever fires), or completes as success with the 
task's work never run. We hit this in production at Patreon on 3.3.1 across 
~109 runs / 55 dags in 48h before finding the cause — details and a 
deterministic reproducer in #72061.
   
   #68244's SKIP LOCKED protects the SELECT, but the list is stale by the time 
the DELETE runs. The window is the full round trip between the two statements, 
and `clean_unused()` runs every triggerer loop tick.
   
   ## How
   
   The two-step exists because MySQL raises error 1093 for a DELETE whose 
subquery selects from the target table (#38663). That restriction is only about 
the target table: correlated NOT EXISTS subqueries against `task_instance`, 
`asset`, and `callback` are legal inside the DELETE. So this change keeps the 
materialized ids as a candidate set (preserving #68244's SKIP LOCKED behaviour 
on the SELECT) and re-checks the same three reference predicates inside the 
DELETE itself. A trigger that gained a reference after the SELECT now survives 
the sweep and gets cleaned on a later pass once it's genuinely unused — which 
is exactly how the single-statement branch already behaves for every other 
dialect.
   
   Verified against real MySQL 8.0: stock code loses rows within one run of a 
200-task deferral-churn dag (~800 defer events); with this change, zero rows 
lost across 21,000+ defer events. Postgres control: zero losses either way.
   
   ## Behaviour change
   
   Only on MySQL: a trigger that becomes referenced between the candidate 
SELECT and the DELETE is no longer deleted (previously it was, cascading into 
the task instance row). No change for other dialects.
   
   ## Test plan
   
   - [x] new regression test 
`test_clean_unused_keeps_triggers_referenced_after_candidate_select`: pins the 
dialect to "mysql" so the two-step branch runs on every backend (on the MySQL 
matrix it exercises the real dialect SQL), interleaves a deferral between the 
id SELECT and the DELETE, and asserts both the trigger and the task instance 
row survive. Fails on current main (`assert 0 == 1` — trigger deleted), passes 
with the fix.
   - [x] existing `test_clean_unused` still passes
   - [x] `prek run --from-ref main`
   
   Could a committer add the `backport-to-v3-3-test` label? The widened 
exposure shipped in 3.3.1 (via #68244), so 3.3.x is the line running this in 
the wild.
   
   ---
   
   **^ Add meaningful description above**
   
   - [x] Yes (please specify the tool below)
   
   Generated-by: Claude Code (investigation, fix, and tests were reviewed, run, 
and reproduced by a human before submission)
   
   Read the **[Pull Request 
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
 for more information.
   


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