Skip to content

Fix deadlock in ti_update_state: FOR UPDATE OF task_instance only#67246

Merged
kaxil merged 1 commit into
apache:mainfrom
avolant:fix/ti-state-update-dag-run-deadlock
May 21, 2026
Merged

Fix deadlock in ti_update_state: FOR UPDATE OF task_instance only#67246
kaxil merged 1 commit into
apache:mainfrom
avolant:fix/ti-state-update-dag-run-deadlock

Conversation

@avolant
Copy link
Copy Markdown
Contributor

@avolant avolant commented May 20, 2026

Problem

PATCH /execution/task-instances/{id}/state calls:

ti = session.get(TI, task_instance_id, with_for_update=True)

The TI mapper has dag_run = relationship(..., lazy="joined"), so this emits:

SELECT task_instance.*, dag_run.*
FROM task_instance
JOIN dag_run ON dag_run.dag_id = task_instance.dag_id
           AND dag_run.run_id  = task_instance.run_id
WHERE task_instance.id = %s
FOR UPDATE

FOR UPDATE with no OF clause locks every relation in the FROM list, both task_instance and dag_run. Under concurrent task completions in the same DAG run, all workers serialise on a single dag_run row, deadlocking with the scheduler's TriggerRuleDep queries (which run in transactions that also touch task_instance).

Observed in production: ~2,000 psycopg2.errors.DeadlockDetected + widespread statement_timeout (5 s) errors per hour on the PATCH .../state endpoint, with a mean execution time of 4,297 ms (pure lock-wait; the query does zero disk I/O).

Three other call sites in this file already use with_for_update(of=TI) for exactly this reason (lines 152, 339, 697). The two remaining bare with_for_update=True calls are the ones hit on every normal task completion.

Fix

Scope the lock to task_instance only:

ti = session.get(TI, task_instance_id, with_for_update={"of": TI})

This produces FOR UPDATE OF task_instance, leaving dag_run unlocked and breaking the deadlock cycle.

Testing

Existing unit tests cover the state-update path. No behaviour change — the function still reads the dag_run joinedload; it just no longer holds a row lock on it.

session.get(TI, id, with_for_update=True) emits a SELECT that joins
dag_run (via the lazy="joined" relationship) and applies FOR UPDATE to
both tables. Under concurrent task completions this serialises all
workers on the same dag_run row, producing deadlock cycles with the
scheduler's trigger-rule dependency checks.

Three other callsites in this file already use with_for_update={"of": TI}
for exactly this reason. Apply the same fix to the two remaining callsites
in _create_ti_state_update_query_and_update_state and its error-recovery
path.
@kaxil kaxil added this to the Airflow 3.2.2 milestone May 21, 2026
@kaxil kaxil added the backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch label May 21, 2026
@kaxil kaxil merged commit 315d159 into apache:main May 21, 2026
138 of 141 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
v3-2-test PR Link

@vatsrahul1001 vatsrahul1001 added the type:bug-fix Changelog: Bug Fixes label May 21, 2026
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ing dag_run (#67246) (#67264)

session.get(TI, id, with_for_update=True) emits a SELECT that joins
dag_run (via the lazy="joined" relationship) and applies FOR UPDATE to
both tables. Under concurrent task completions this serialises all
workers on the same dag_run row, producing deadlock cycles with the
scheduler's trigger-rule dependency checks.

Three other callsites in this file already use with_for_update={"of": TI}
for exactly this reason. Apply the same fix to the two remaining callsites
in _create_ti_state_update_query_and_update_state and its error-recovery
path.
(cherry picked from commit 315d159)

Co-authored-by: Arthur <arthur.volant@datadoghq.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ing dag_run (#67246) (#67264)

session.get(TI, id, with_for_update=True) emits a SELECT that joins
dag_run (via the lazy="joined" relationship) and applies FOR UPDATE to
both tables. Under concurrent task completions this serialises all
workers on the same dag_run row, producing deadlock cycles with the
scheduler's trigger-rule dependency checks.

Three other callsites in this file already use with_for_update={"of": TI}
for exactly this reason. Apply the same fix to the two remaining callsites
in _create_ti_state_update_query_and_update_state and its error-recovery
path.
(cherry picked from commit 315d159)

Co-authored-by: Arthur <arthur.volant@datadoghq.com>
kaxil added a commit that referenced this pull request May 22, 2026
…mit (#67353)

PR #59686 dropped the _handle_fail_fast_for_dag call in the MySQL-TIMESTAMP-limit
branch of the reschedule path based on an incorrect SQLA2 deadlock concern. As a
result, DAGs with fail_fast=True silently fail to stop sibling tasks when a
reschedule date exceeds 2038-01-19 on MySQL.

The actual deadlock that motivated #59686 came from a different path (FOR UPDATE
expanding to the lazy-joined dag_run row), fixed in #67246 by scoping the lock
with with_for_update={"of": TI}. With that scope in place, the fail-fast call is
safe and matches the file's two existing fail-fast sites.

Also drops a second misleading comment in the same function claiming session.get
was avoided to "avoid SQLA2 lock contention issues" -- the code itself is fine;
the rationale was wrong.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:task-sdk backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants