Phinhas214 commented on PR #68705: URL: https://github.com/apache/airflow/pull/68705#issuecomment-6301930606
Thanks for this. I tested the PR head in Breeze on SQLite and PostgreSQL 14. I also reproduced #68699 on 3.2.2 with its setup (the issue's Dag, 1096 daily runs, concurrent `POST /api/v2/backfills`). The single transaction using `session.flush()` fixes the orphan for every error, not only lock errors. With the PR I saw no orphaned or partial backfills. The model test shows this well: it raises a `RuntimeError`, which the old `_cleanup_partial_backfill` helper did not handle. **question (non-blocking):** You note that the `num_active` race predates this PR and that the flush widens the window. I confirmed it, and the visible effect changes. Late requests now wait for the earlier transaction and then succeed with **HTTP 200**. They create active backfills whose runs all have `exception_reason = 'in flight'` and no Dag runs. So the user sees success for backfills that will never run anything. I agree that the guard can be a separate change to patch this issue up. Can you open a follow-up issue for it and link it here or I can open a PR for it (which ever one is more comfortable for you). - One note for that change: MySQL has no partial indexes (see the comment at `idx_dag_run_queued_dags` in `dagrun.py`), so a partial unique index would need a different design there. A row lock on the `DagModel` row works on PostgreSQL and MySQL 8, but not on SQLite, since SQLIte ignores `FOR UPDATE`. -- 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]
