Fix dag_tag unique-constraint crash when multiple dag processors parse concurrently#70002
Fix dag_tag unique-constraint crash when multiple dag processors parse concurrently#70002kjh0623 wants to merge 2 commits into
Conversation
|
@kjh0623 This PR has been converted to draft because it does not yet meet our Pull Request quality criteria. Issues found:
What to do next:
Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
…e concurrently With multiple dag processors running in HA, two processors can parse the same file concurrently: both read the (empty or stale) tags relationship, then both INSERT the same dag_tag row. The plain ORM INSERT raised UniqueViolation, poisoning the session and surfacing as PendingRollbackError on the retry, crashing the dag processor. Insert tags with a dialect-aware conflict-ignoring statement instead (ON CONFLICT DO NOTHING on PostgreSQL/SQLite, ON DUPLICATE KEY UPDATE no-op on MySQL), following the existing pattern used for AssetActive in the same module. Pending tag DELETEs are flushed before the INSERT, which also preserves the MySQL case-only rename behaviour previously special-cased (apache#57113), so that block is folded into the new flow. The insert is batched across all dags in update_dags so bulk_write_to_db keeps its constant query count (the per-dag flush+insert variant tripped the assert_queries_count guards in test_dag.py).
eaaf359 to
21e6032
Compare
With multiple dag processors running in HA (e.g.
replicas: 2), two processors can parse the same file concurrently: both read the (empty or stale)DagModel.tagsrelationship, then both INSERT the samedag_tagrow. The plain ORM INSERT raises a unique-constraint violation (psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "dag_tag_pkey"), which poisons the session, surfaces asPendingRollbackErroron the retry, and crashes the dag processor.We hit this in production while running two dag processors over ~2,900 DAGs and have been running this fix as an internal patch since; this upstreams it.
Fix
_update_dag_tagsnow inserts tags with a dialect-aware conflict-ignoring statement —ON CONFLICT DO NOTHINGon PostgreSQL/SQLite, self-assignON DUPLICATE KEY UPDATEon MySQL — following the existing pattern used forAssetActivein the same module (activate_assets_if_possible).update_dags(the helper now takes the whole batch), sobulk_write_to_dbkeeps its constant query count — theassert_queries_countguards intest_dag.pystay at their current values.(name, dag_id)primary key), so the MySQL special-case block from Fix dag-processor crash when renaming DAG tag case on MySQL #57113 is folded into the new flow, and it ensures not-yet-flushedDagModelrows exist to satisfy thedag_tagFK.bulk_write_to_dbalready re-fetches the dags withjoinedload(DagModel.tags)right afterupdate_dags, so this adds no extra queries.Tests
_update_dag_tagsruns — previouslyIntegrityError, now tolerated with the remaining tags still applied.DagModelis flushed before the tag insert (FK safety).test_update_dag_tagscases (including MySQL case-only renames) cover the folded-in Fix dag-processor crash when renaming DAG tag case on MySQL #57113 behaviour across all backend matrices.closes: #68263
related: #50815
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code following the guidelines
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.