xchwan commented on code in PR #75708:
URL: https://github.com/apache/airflow/pull/75708#discussion_r4521211293
##########
airflow-core/src/airflow/serialization/definitions/dag.py:
##########
@@ -861,6 +862,15 @@ def _process_dagrun_deadline_alerts(
)
stats.incr("deadline_alerts.deadline_creation_failed",
tags=metrics_tags)
+ # An alert can roll the session back, discarding the pending
DagRun. Carrying on would
+ # add Deadlines that violate the dagrun_id foreign key, and
returning normally would let
+ # the caller treat the missing run as created.
+ if orm_dagrun not in session:
Review Comment:
Implemented and tested on PostgreSQL, MySQL and SQLite.
- Alerts are evaluated on their own session. It is never committed, and a
snapshot of the run (`dag_id`, `logical_date`, `queued_at`) is passed instead
of the pending ORM object. `Deadline` rows are still added on the scheduler's
session.
- A rollback inside an alert now only affects that session: the alert is
skipped, the run is created and other Dags' runs in the same tick survive. The
`orm_dagrun not in session` check and `DagRunRolledBackError` stay as a
backstop for references that reach the scheduler's session on their own.
- The alert session is read-only. The scheduler holds locks on the Dag's
rows while it creates the run, so a reference that writes or runs `SELECT ...
FOR UPDATE` on the new session waits on a lock held by the scheduler, which is
itself waiting on the reference. I reproduced it: the scheduler hung
indefinitely on PostgreSQL, for 50 seconds on MySQL and for about 5 seconds on
SQLite. With the read-only session the statement fails at once and only that
alert is skipped.
- `deadline-alerts.rst` now says a reference should only run read-only
queries on the session it is given and should not open its own.
Behavior change: writing through that session used to be committed with the
scheduler's transaction. The docs only promised read queries, so I treated it
as unsupported rather than keeping it.
Checked with the plugin and Dag from the issue, the real scheduler and API
server, `dags test` and a backfill: on main the scheduler exits (or the API
returns 500); with this change it keeps running and every run is created.
---
Drafted-by: Claude Code (Sonnet 5.5); reviewed by @xchwan 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]
