Repository navigation
Roll back session between retries in Trigger.clean_unused - #74650
RishabhBajaj97 wants to merge 2 commits into
Conversation
A transient metadata-database disconnect raises DBAPIError and leaves the SQLAlchemy session in a pending-rollback state. The Tenacity retry wrapper inside Trigger.clean_unused then reuses the same session on the next attempt and raises PendingRollbackError, which the retry policy does not match, so the triggerer process exits. The in-flight deferred triggers it was polling must be reclaimed by another triggerer, interrupting their asyncio state and restarting any mid-poll progress. Mirror the pattern already used by retry_db_transaction: on DBAPIError or StaleDataError inside the retry attempt, call session.rollback() before re-raising so Tenacity drives the next retry on a clean session.
|
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
|
|
#70961 merged today and rewrote Should this one be closed, with a note on #74614 that #70961 fixed it? I ran a test in Breeze on SQLite that closes the DBAPI connection under the task_instance UPDATE: Drafted-by: Claude Code 2.1.302; reviewed by @luc-pimentel before posting |
When the metadata database has a transient disconnect (RDS/Aurora failover, idle-connection drop by the NLB, routine Postgres maintenance), the
UPDATE task_instanceinsideTrigger.clean_unusedraisesDBAPIError. Therun_with_db_retries()wrapper is supposed to absorb this, but it only matchesDBAPIError/StaleDataError— and after aDBAPIError, the SQLAlchemy session is left in a pending-rollback state. The next retry attempt reuses that invalidated session and raisesPendingRollbackError, which the retry policy does not match. The exception propagates out ofclean_unused, the triggerer process exits, and every in-flight deferred trigger it was polling has to be reclaimed by another triggerer, losing its asyncio progress.The project already has the right pattern in
retry_db_transaction(airflow/utils/retries.py), which catchesDBAPIError/StaleDataErrorinside the retry attempt, callssession.rollback(), and re-raises so Tenacity drives the next retry on a clean session.retry_db_transactioncan't be applied directly because its docstring says it must not be used with@provide_session(andclean_unusedalready is), so this PR mirrors the pattern inline.Validated:
test_clean_unused_rolls_back_session_on_transient_db_errorpasses with the fix (1 passed in 10.20s) and fails without it (AssertionError: Expected 'rollback' to have been called once. Called 0 times.).airflow-core/tests/unit/models/test_trigger.pysuite: 49 passed, no regressions.ruff format+ruff checkclean.mypy airflow-core/src/airflow/models/trigger.pyrun inside Breeze:Success: no issues found in 1 source file(host-side mypy prek hook can't set up its venv on this machine due to a glibc mismatch unrelated to the diff).prek run --from-ref main --stage pre-commit) all pass for the matched files;mypy-airflow-core+mypy-devel-commonskipped locally and verified via Breeze as noted above.Related: #70961 is an in-review PR that batches
clean_unused; its restructure towith create_session(scoped=False)per retry attempt would incidentally close this issue as a side effect of the rework. This PR is a smaller, targeted fix in case the batching PR takes longer to land.closes: #74614
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines