ukiband commented on issue #71923: URL: https://github.com/apache/airflow/issues/71923#issuecomment-5433147603
Reporter here. I went through both #71934 and #71966 against the timeline from our incident. A few notes from our side that may help: **Mechanism**: Both PRs identify the root cause correctly. The listener hooks currently run between the `task_reschedule` DELETE in `prepare_db_for_next_try` and the `ti.id` UPDATE flushed by `save_to_db`. A slow listener stretches that gap from adjacent statements to several seconds. **Is narrowing the window sufficient?** I believe so. An otherwise identical deployment without the listener survived the same rolling restart without crashing. Without a listener, the gap collapses to the duration between two adjacent SQL statements. While I mentioned more invasive options in the original issue (row locking, or catching `IntegrityError` in the scheduler loop), I don't think they are warranted now. They touch the scheduler and Execution API hot paths and would introduce new risks for an extremely rare race condition. **Comparison of the PRs**: They are not mutually exclusive. - #71966 is the minimal change. Moving the listener call before `prepare_db_for_next_try` closes the widened window entirely. Having the listener see the id of the try that actually failed also looks like the right behavior. I checked that `TaskInstanceHistory.record_ti` now runs with `state=UP_FOR_RETRY` instead of `RUNNING`; since `record_ti` records any non-finished state as `FAILED`, the history row is unchanged. - #71934 adds a defensive second DELETE right before the flush, along with a test that exercises the scenario. The test simulates the concurrent insert from within the listener hook in the same session, so it checks the second DELETE rather than true cross-transaction concurrency, but that is a reasonable proxy here. **Recommendation**: Combining the reordering from #71966 with the test from #71934 seems like the cleanest path forward, though I'll leave the final call to the maintainers. **Note on our setup**: We are running Airflow 3.1.8 and understand a fix won't land on the 3.1 release line. In the meantime we are mitigating this by lowering `[openlineage] execution_timeout`. A backport to 3.3.x would be appreciated if feasible. -- 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]
