XD-DENG commented on PR #64076: URL: https://github.com/apache/airflow/pull/64076#issuecomment-4120674262
I wonder if adding a bare `session.commit()` inside a `@provide_session`-decorated function might have some unintended side effects: 1. **Session contract** — generally, functions with a `session` parameter don't call `session.commit()` themselves, since callers could pass their own session expecting transactional atomicity. 2. **Retry semantics** — since this function is wrapped in `run_with_db_retries`, a mid-function commit would mean the `except OperationalError` rollback can only undo the second phase. The already-committed UPDATE on the `job` table wouldn't be rolled back, which could affect the atomic retry guarantee. The goal of releasing locks on the `job` table early makes a lot of sense though\! One possible alternative could be splitting this into two separate `@provide_session` methods — one for marking stale jobs as FAILED, one for the TI adoption/reset — each with its own session and retry scope. What do you think? -- 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]
