potiuk commented on code in PR #68125:
URL: https://github.com/apache/airflow/pull/68125#discussion_r3681643023
##########
airflow-core/src/airflow/models/backfill.py:
##########
@@ -640,7 +640,13 @@ def _create_backfill(
if not serdag:
raise DagNotFound(f"Could not find dag {dag_id}")
- dag_model = session.scalar(select(DagModel).where(DagModel.dag_id ==
dag_id).limit(1))
+ dag_model = session.scalar(
Review Comment:
The lock only exists if this returns a row — the `if dag_model:` below means
a `dag_id` with a serialized Dag but no `DagModel` row proceeds with no lock at
all, and the duplicate-backfill race is back for that case.
It's a narrow window and arguably shouldn't happen (the `serdag` lookup
above already raises `DagNotFound`), but the protection currently reads as
unconditional while it isn't. Either assert the row exists before proceeding,
or add a comment noting the case is impossible and why.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
airflow-core/src/airflow/models/backfill.py:
##########
@@ -699,12 +705,7 @@ def _create_backfill(
triggering_user_name=triggering_user_name,
)
session.add(backfill)
- # Commit immediately so the backfill is visible to concurrent requests
- # checking num_active backfills, preventing duplicate active backfills
- # for the same dag.
- session.commit()
-
- session.scalars(select(DagModel).where(DagModel.dag_id ==
dag_id)).one()
+ session.flush()
Review Comment:
Worth a short comment here explaining *why* the flush replaces the old
commit — that the `DagModel` row lock taken above is what now serialises
concurrent creators, so the backfill no longer needs to be committed to be
visible.
The removed comment explained the old reasoning; without a replacement, the
next reader sees a bare `flush()` and may well "fix" it back into a `commit()`.
One line at the source of truth is enough.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
