Skip to content
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

Fix slow DAG deletion due to missing dag_id index for job table #20282

Merged
merged 2 commits into from
Dec 30, 2021
Merged

Fix slow DAG deletion due to missing dag_id index for job table #20282

merged 2 commits into from
Dec 30, 2021

Conversation

kushsharma
Copy link
Contributor

Fixes #20249

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Dec 14, 2021
@kushsharma kushsharma force-pushed the index-dag_id-in-job branch 2 times, most recently from 19b8013 to ddbbdd9 Compare December 14, 2021 05:26
@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 14, 2021
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@kushsharma kushsharma requested a review from potiuk as a code owner December 14, 2021 08:41
@kushsharma
Copy link
Contributor Author

@uranusjr can you check what version I should use in the migration docs(I am not sure about it)? I am just using 2.3.0 for now as one of the tests requires me to update that file.

potiuk
potiuk approved these changes Dec 14, 2021
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. So it turned out to be just one index missing :). Cool.

@potiuk
Copy link
Member

potiuk commented Dec 14, 2021

Static checks need fixing and I think this change uses an old migration head (some change likely implemented hen you branched off?)

@uranusjr
Copy link
Member

2.3.0 should work. But yeah you should change the down_version and the corresponding column in migration-ref to 786e3737b18f instead. And fix the static check failure.

@kaxil kaxil added this to the Airflow 2.3.0 milestone Dec 30, 2021
kushsharma and others added 2 commits December 30, 2021 01:10
@kushsharma @kaxil
Add dag_id index for job table
63662c3 
- helps in speeding up delete operation required to query jobs by dag_id

Signed-off-by: Kush <[email protected]>
@kaxil
Update 587bdf053233_adding_index_for_dag_id_in_job.py
4335b24
@kaxil kaxil force-pushed the index-dag_id-in-job branch from 5751f12 to 4335b24 Compare December 30, 2021 01:12
kaxil
kaxil approved these changes Dec 30, 2021
View reviewed changes
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have rebased on main, resolved the conflicts and updated the down revision

@kaxil kaxil merged commit ac9f29d into apache:main Dec 30, 2021
@jedcunningham jedcunningham mentioned this pull request Feb 15, 2022
Merged
@jedcunningham jedcunningham added the type:bug-fix Changelog: Bug Fixes label Feb 15, 2022
jedcunningham pushed a commit that referenced this pull request Feb 16, 2022
@kushsharma @jedcunningham
Fix slow DAG deletion due to missing dag_id index for job table (#…
045f6ce 
…20282)

Fixes #20249

(cherry picked from commit ac9f29d)
jedcunningham pushed a commit that referenced this pull request Feb 17, 2022
@kushsharma @jedcunningham
Fix slow DAG deletion due to missing dag_id index for job table (#…
436f452 
…20282)

Fixes #20249

(cherry picked from commit ac9f29d)
@jedcunningham jedcunningham mentioned this pull request Feb 18, 2022
Closed
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaxil kaxil kaxil approved these changes

@uranusjr uranusjr uranusjr approved these changes

@potiuk potiuk potiuk approved these changes

@ashb ashb Awaiting requested review from ashb ashb is a code owner

@XD-DENG XD-DENG Awaiting requested review from XD-DENG XD-DENG is a code owner

Assignees
No one assigned
Labels
area:Scheduler including HA (high availability) scheduler full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Milestone
Airflow 2.2.4
Development

Successfully merging this pull request may close these issues.

DAG deletion is slow due to lack of database indexes on dag_id
5 participants
@kushsharma @potiuk @uranusjr @kaxil @jedcunningham