Cleanup leftovers in packages_dependencies table
What does this MR do and why?
MR !20549 (merged) added two tables: packages_dependencies
and packages_dependency_links
. When a package is deleted, packages_dependency_links
rows are properly deleted but rows in packages_dependencies
are kept and they can become orphans. This MR aims to clean up those orphaned packages_dependencies
records.
How to set up and validate locally
- In Rails console, create a bunch of orphaned
packages_dependencies
records:FactoryBot.create_list(:packages_dependency, 100)
- Enable the feature flag :
Feature.enable(:packages_delete_orphaned_dependencies_worker)
- Run the cleanup sidekiq worker:
Packages::Cleanup::DeleteOrphanedDependenciesWorker.perform_async
, the orphaned records should be deleted.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Database analysis
Related to #38266 (closed)
Merge request reports
Activity
changed milestone to %Backlog
assigned to @mkhalifa3
4 Warnings This MR has a Changelog commit for EE, but no code changes in ee/
. Consider removing theEE: true
trailer from your commits.89de29c4: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. This merge request has more than 20 commits which may cause issues in some of the jobs. If you see errors like missing commits, please consider squashing some commits so it is within 20 commits. Please add a merge request subtype to this merge request. Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Siddharth Dungarwal (
@sdungarwal
) (UTC+5.5)George Koltsov (
@georgekoltsov
) (UTC+1)database Leonardo da Rosa (
@l.rosa
) (UTC-3)Adam Hegyi (
@ahegyi
) (UTC+2)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Sidekiq queue changes
This merge request contains changes to Sidekiq queues. Please follow the documentation on changing a queue's urgency.
These queues were added:
cronjob:packages_cleanup_delete_orphaned_dependencies
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Dangermarked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
requested review from @dmeshcharakou
- Resolved by Adam Hegyi
- Resolved by David Fernandez
Thanks @mkhalifa3!
Can you check how many records from
packages_dependencies
table we're going to delete when we run the worker for the first time.Several database queries will need a database analysis. You can check the guide on how to do that.
As soon as you have them you can assign database label to MR and danger bot will suggest database reviewers.
Please pay attention to the danger bot
and correct what is possible.
removed review request for @dmeshcharakou
- Resolved by Moaz Khalifa
- Resolved by Moaz Khalifa
- Resolved by Moaz Khalifa
- Resolved by Moaz Khalifa
- Resolved by Moaz Khalifa
- Resolved by Adam Hegyi
- Resolved by Adam Hegyi
- Resolved by Adam Hegyi
- Resolved by Moaz Khalifa
added 467 commits
-
2b8aace0...112ed929 - 465 commits from branch
master
- 24cbd43a - Reduce BATCH_SIZE to 1000
- 11901853 - Merge branch 'master' into 38266-Cleanup-leftovers-in-packages_dependencies-table
-
2b8aace0...112ed929 - 465 commits from branch
added database label
- A deleted user
added databasereview pending label
Hi @OmarQunsulGitlab can you plz do the
database
review for this MR? thanks in advance!requested review from @OmarQunsulGitlab
- Resolved by Adam Hegyi
I'm out of the capacity and will be on PTO from 3.03 to 8.03
Feel free to re-assign if you want to move it forward before I'm back.
- Resolved by Moaz Khalifa
- Resolved by Moaz Khalifa
- Resolved by Omar Qunsul
@mkhalifa3 I added some backend and database comments. I still need to get back to the queries later, because some of them might be problematic, and we need to reduce their time. In the meanwhile let me know what you think about my comments.
btw, I feel that this somehow reinvents what
LooseForeignKeys
does, and it might benefit greatly from it.You still need to do the first clean up though. And this might be done with a batched background migration for example.
mentioned in merge request !113556 (merged)
removed review request for @OmarQunsulGitlab
- Resolved by Adam Hegyi
added 1292 commits
-
11901853...e1689e80 - 1289 commits from branch
master
- 5cf01172 - Merge branch 'master' into 38266-Cleanup-leftovers-in-packages_dependencies-table
- 6bd8a801 - Merge branch 'master' into 38266-Cleanup-leftovers-in-packages_dependencies-table
- ca899ce7 - Convert DeleteOrphanedDependenciesWorker to a LimitedCapacityWorker
Toggle commit list-
11901853...e1689e80 - 1289 commits from branch
added 1 commit
- 38e0e1c9 - Add DeleteOrphanedDependenciesWorker to retry_exceptions
mentioned in issue gitlab-com/www-gitlab-com#14098 (closed)
- Resolved by Adam Hegyi
- Resolved by Adam Hegyi
- Resolved by Adam Hegyi
- Resolved by Adam Hegyi