Fix scheduler dag-version refresh deadlock with PK-ordered locking#66762
Draft
capytan wants to merge 1 commit into
Draft
Fix scheduler dag-version refresh deadlock with PK-ordered locking#66762capytan wants to merge 1 commit into
capytan wants to merge 1 commit into
Conversation
boring-cyborg
Bot
added
the
area:Scheduler
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
capytan
force-pushed
the
fix/verify-integrity-dag-version-deadlock
branch
2 times, most recently
from
99888c3 to
e478a3f
Compare
Replace the bulk UPDATE in _verify_integrity_if_dag_changed (which locked task_instance rows via the (dag_id, run_id) index) with a SELECT id ORDER BY id FOR UPDATE SKIP LOCKED + UPDATE WHERE id IN (...) batch loop. The previous bulk UPDATE could deadlock with PK-ordered writers - notably the api-server ti_update_state path, which locks rows by primary key under concurrent task state updates; locking in PK order eliminates the cross-index race. Filter the SELECT on dag_version_id so each batch advances past already-updated rows. Without this guard the batch loop never terminates: state IN (State.unfinished) still matches the rows that the previous batch just tagged with the latest dag_version_id, so SELECT keeps returning the same first N task_instance ids. With BATCH_SIZE=1000 the bug only surfaces for dag_runs that have more than one batch of unfinished task instances, which is why it slipped through the default tests. Add test_verify_integrity_processes_more_than_one_batch which uses monkeypatch to shrink _DAG_VERSION_REFRESH_BATCH_SIZE to 2 and asserts every initial task instance ends up tagged with the latest dag_version after the refresh.
capytan
force-pushed
the
fix/verify-integrity-dag-version-deadlock
branch
from
e478a3f to
211c4f3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
related: #66734
After upgrading to 3.2.1 we hit periodic Postgres deadlocks on
task_instancebetween two writers that lock the same rows in different orders:_verify_integrity_if_dag_changedissuesUPDATE task_instance ... WHERE dag_id=? AND run_id=? AND state IN (...), which the planner satisfies via the(dag_id, run_id)indexti_update_statedoesSELECT FOR UPDATEon the same rows by primary keyWhen the row sets overlap, PG aborts one of them with
deadlock detected. Traceback and pg_stat_activity output are in #66734.This PR replaces the bulk UPDATE with a
SELECT id ORDER BY id LIMIT N FOR UPDATE SKIP LOCKED+UPDATE WHERE id IN (...)batch loop, so the scheduler also locks in PK order. Same approach #65920 takes forcheck_trigger_timeouts; #65818 tracks this family of bugs.One thing that bit me on the way: the inner UPDATE only changes
dag_version_id, notstate, so a SELECT filtered solely onstate IN (State.unfinished)keeps returning the same N ids on every iteration and the loop never terminates once the batch is full. The candidate SELECT also filters ondag_version_id IS NULL OR != latest_dag_version.idso each batch advances. The previous single bulk UPDATE never surfaced this; the batched form needs it.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Opus 4.7 (1M context) following the guidelines