Repository navigation
Conversation
Let the owning transaction retry the original database exception instead of continuing with a failed Session and raising PendingRollbackError. Cover transient failures, retry exhaustion, and real MySQL deadlock recovery without changing HA commit guards.
|
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
|
|
Could a maintainer please review and approve the pending CI workflows? All three workflows are awaiting approval and have not started. This PR only changes scheduler error propagation and regression tests; it does not modify workflow files. Thank you. |
SEPURI-SAI-KRISHNA
left a comment
SEPURI-SAI-KRISHNA
left a comment
There was a problem hiding this comment.
Ran the patch locally since the GitHub Actions workflows have not run yet. On SQLite your four non-MySQL tests pass and the two MySQL ones skip. Reverting only the production change and keeping the tests, all four fail with PendingRollbackError after the original IntegrityError, so this reproduces on SQLite and is not MySQL specific. Full test_scheduler_job.py: 473 passed before the patch, 477 after, with the same pre-existing 2 failures (statsd missing) and 75 errors (airflow_shared missing) in both.
Coverage looks complete. _create_dagruns_for_dags is already decorated @retry_db_transaction and run_with_db_retries rolls the session back before each attempt, so the recovery path existed and the per-Dag handlers were making it unreachable. _create_dag_runs_asset_triggered has no except around its create_dagrun, and both paths reach _get_current_dag, which the first hunk covers.
One gap: run_with_db_retries retries on (DBAPIError, StaleDataError), but both new handlers re-raise DBAPIError only. StaleDataError is not a DBAPIError subclass, so it still lands in the broad except Exception and leaves the session in the state this PR fixes. It appears nowhere in scheduler_job_runner.py today so I have no reproduction, but if the boundary retries it the handler should too: except (DBAPIError, StaleDataError): raise.
ORM flush failures can leave the Session unusable just like driver errors. Keeping both retryable error categories visible to the owning transaction lets its existing rollback and retry policy recover the batch instead of masking the original failure.
|
Thanks for catching the Both handlers now propagate With only the production handlers restored to their pre-review version, all four new cases fail with
|
Database and ORM flush errors while loading a serialized Dag or creating a scheduled DagRun are caught by per-Dag handlers. Continuing with the failed SQLAlchemy Session can mask the original retryable error with
PendingRollbackErrorand interrupt scheduling.Propagate both
DBAPIErrorandStaleDataErrorto the existing_create_dagruns_for_dagsretry boundary, matching the error categories handled byretry_db_transaction. The owner rolls back before re-querying candidates and reacquiring their row locks. Ordinary per-Dag error handling, retry limits, andCommitProhibitorGuardremain unchanged. Persistent errors still propagate after retry exhaustion; this does not provide per-Dag isolation for persistent failures.Regression tests cover transient failures and retry exhaustion at both interception points using real failed flushes: a duplicate Pool constraint violation and a zero-row Pool UPDATE that raises SQLAlchemy's
StaleDataError. They verify the original exception on exhaustion, bounded retries, an active Session after rollback, successful DagRun creation after recovery, and restoration of the Pool data.The MySQL-only test uses two real InnoDB transactions to cause deadlock 1213 during pending Dag metadata flush, with autoflush enabled and disabled, after a DagRun has already been created in the batch. It checks a fresh locked candidate query, exactly one run per scheduled Dag, and preservation of concurrent metadata updates.
Related: #59120
Validation for the StaleDataError follow-up, through Breeze on Python 3.11 with the official Linux amd64 CI image:
PendingRollbackErrorafter the native zero-row UPDATE errorFor the initial contribution, reverting the DBAPIError propagation made all 6 original MySQL cases fail with
PendingRollbackErrorafter the original constraint error or deadlock. The initial broader selective checks had seven failures that were independently reproduced without the PR: five plugin-count failures and two LangSDK file-descriptor failures. Those broader selective checks and documentation build have not been rerun for this follow-up; the initial documentation build passed.These tests exercise the transaction boundary directly and do not establish multi-scheduler process acceptance.
Was generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-6) following the guidelines