SEPURI-SAI-KRISHNA commented on issue #59120: URL: https://github.com/apache/airflow/issues/59120#issuecomment-6165103303
Not working on it, so nothing to coordinate around on my side. On the savepoint route you referenced: it is blocked, and the reason is worth knowing before you spend time there. `_create_dag_runs` runs inside `prohibit_commit`, and `CommitProhibitorGuard` listens on `before_commit`, which SQLAlchemy dispatches for nested transactions too (`if self._parent is None or self.nested`). So releasing a `SAVEPOINT` raises `UNEXPECTED COMMIT - THIS WILL BREAK HA LOCKS!`. Dropping to `session.connection().begin_nested()` sidesteps the Session event layer but leaves the Session in `PendingRollbackError` after the first failure, so that is a dead end too. Details and a variant that works are in https://github.com/apache/airflow/issues/59120#issuecomment-5077987321, re-checked on SQLAlchemy 2.0.52. That variant needs a change to `CommitProhibitorGuard` itself, and whether widening a core primitive is acceptable is the question I left open here and never got answered. Your direction avoids that entirely: propagating `DBAPIError` to the existing `run_with_db_retries` boundary reuses machinery that already rolls back before retrying, and leaves the guard alone. I think that is the better shape for exactly that reason. The piece this issue has never had is a reproduction on a real MySQL backend. My own check was on SQLite, which does not reproduce the aborted-transaction state, so it established the guard mechanics and nothing about the actual failure. If you add that regression test it would settle the part that analysis cannot. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
