Skip to content

Fix bad delete logic for dagruns #32684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 19, 2023
Merged

Fix bad delete logic for dagruns #32684

merged 5 commits into from
Jul 19, 2023

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish requested a review from jedcunningham July 18, 2023 21:59
@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Jul 18, 2023
jedcunningham
jedcunningham reviewed Jul 18, 2023
View reviewed changes
dstandish and others added 3 commits July 19, 2023 09:53
@dstandish dstandish merged commit 7092cfd into apache:main Jul 19, 2023
@dstandish dstandish deleted the fix-bad-delete-logic-for-dagruns branch July 19, 2023 20:27
@pankajkoti pankajkoti mentioned this pull request Jul 21, 2023
Closed
2 tasks
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Aug 2, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.0 milestone Aug 2, 2023
@potiuk potiuk mentioned this pull request Aug 3, 2023
Closed
2 tasks
@RNHTTR RNHTTR mentioned this pull request Aug 21, 2023
Closed
2 tasks
@hussein-awala
Copy link
Member

I spent all day debugging to figure out how our ITs disappear from all dags at the same time, and why most of the dags were stuck (we're using depend_on_past), and after many analyzes of the log table and the RDS query history in datadog, I found that the query that should remove TIs from 2 dag runs removed over 2k TIs.

Luckily I found this PR which confirms my hypothesis. Thank you for this fix @dstandish!

However, I wonder if we can improve our release notes by adding a level for the bug and its impact on the whole stack. For example, this bug is very important and it could have a significant impact on the whole platform, despite this, it is added in the middle of the bug list with a generic commit message:

Fix bad delete logic for dagruns (#32684)

@RNHTTR
Copy link
Contributor

RNHTTR commented Aug 31, 2023

Yeah this took me many hours to find :( I wonder if we should add a newsfragment?

hussein-awala added a commit to leboncoin/airflow that referenced this pull request Sep 1, 2023
@hussein-awala
Fix the bad delete logic for dagruns which leads to delete TIs from o…
359939b 
…ther dags (apache#32684)
@potiuk
Copy link
Member

potiuk commented Sep 4, 2023

And we have another similar issue from the user who suffered from it I think https://apache-airflow.slack.com/archives/CCQ7EGB1P/p1693805145064939