goingforstudying-ctrl opened a new pull request, #75946: URL: https://github.com/apache/airflow/pull/75946
Ran into #64534 while looking at why `_mark_backfills_complete` still needs its 2-minute "initializing" heuristic: `_create_backfill` commits the `Backfill` row first and only then creates the `DagRun`/`BackfillDagRun` rows, so for a window the scheduler can see a backfill with zero runs and the API can see it as "active". #62561 guarded the scheduler side with a time-based cutoff, but the window itself never went away. This takes the approach Kaxil sketched in https://github.com/apache/airflow/pull/62561#discussion_r2880363898: `session.commit()` -> `session.flush()`, so the backfill only becomes visible when `create_session()` commits at the end. The row and its runs now appear in one transaction and the race window is gone. The early commit wasn't only for visibility, though. It's also what made concurrent create requests for the same Dag fail with `AlreadyRunningBackfill` — without it, two requests can both pass the `num_active` check. To keep that invariant, creation now locks the Dag row at the *end* (`with_row_locks`, same wrapper `start_drain` uses) and re-checks for another active backfill for the Dag right before commit. A concurrent creator blocks on the lock, then sees the committed row and raises `AlreadyRunningBackfill` instead of committing a duplicate. The lock is taken at the end on purpose: the Dag row stays available to the scheduler while the runs are created, which is the behavior `test_create_backfill_keeps_dag_row_available_until_runs_are_created` pins down. Side benefit: if run creation fails partway (any exception, not just a lock error), the whole thing rolls back now. Before this, a non-lock failure left a committed backfill with no runs behind, and since it never completed it blocked every future backfill for that Dag until someone cleaned it up by hand. The `_cleanup_partial_backfill` helper that only covered the lock-error case is obsolete, so it's removed along with its three dedicated tests; there's an API-level test for the rollback instead. One honest caveat: on MySQL `with_row_locks` doesn't emit a lock (same as the rest of the codebase), so duplicate prevention there degrades to the final re-check alone. That leaves roughly the sub-millisecond window the early commit had, so no worse than today, and Postgres gets the full serialization. Tests: three new ones at the model level (creation commits in a single transaction; a mid-creation failure leaves no orphan rows; a competitor committing mid-creation aborts ours with `AlreadyRunningBackfill` — that one needs real row locks so it skips SQLite). `test_backfill.py`, the backfill API/CLI suites, and the scheduler `_mark_backfills_complete` tests all pass locally. related: #64534 -- 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]
