Preserve concurrent task changes when triggers report results - #72849
Draft
hussein-awala wants to merge 1 commit into
Draft
Preserve concurrent task changes when triggers report results#72849hussein-awala wants to merge 1 commit into
hussein-awala wants to merge 1 commit into
Conversation
A late trigger event or failure could overwrite a task that another transaction had cleared, failed, resumed, or attached to a different trigger. Terminal events could also send callbacks and XComs for a transition that had already lost the race.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The correctness lock is silently disabled when scheduler row-level locking is configured off.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Prevents late trigger results from overwriting concurrent task transitions.
Changes:
- Locks and refreshes deferred task instances before processing trigger results.
- Adds regression coverage for locking, cache refresh, and concurrent transitions.
File summaries
| File | Description |
|---|---|
airflow-core/src/airflow/models/trigger.py |
Centralizes locked deferred-task selection. |
airflow-core/tests/unit/models/test_trigger.py |
Adds concurrency and locking tests. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .execution_options(populate_existing=True) | ||
| ) | ||
| # Wait for concurrent transitions; skipping a locked row would lose this result. | ||
| return session.scalars(with_row_locks(query, session, of=TaskInstance)) |
Member
Author
There was a problem hiding this comment.
That's a good point!
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
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.
Late trigger results could overwrite a concurrent task transition because event and failure submission selected deferred tasks without locking them. A task cleared, failed, resumed, or attached to a replacement trigger after that read could be scheduled again, or receive stale terminal callbacks and XComs.
Lock matching task instances in a consistent order and refresh their cached values before applying an event or failure. Both submission paths share the selection logic. Locks cover only task instances and wait for competing transactions, allowing the database to recheck the deferred state and trigger ID before dispatch.
Validation through Breeze with Python 3.10:
Selective checks request the full core CI matrix, which was not run locally.
Related to #71391, which retries transient deadlocks; this change prevents stale task updates.
Was generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-6) following the guidelines