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.

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

Fixes #21080 - exit out after foreign key error #6971

Merged
merged 1 commit into from Sep 25, 2017

Conversation

beav
Copy link
Contributor

@beav beav commented Sep 22, 2017

Previously, we attempted to gracefully handle foreign key errors that
usually are caused by client checkins during the cleanup script's run.
This "graceful" handling had a bug that caused it to error on every
subsequent batch.

Instead, simply exit and tell the user to re-run the script.

Previously, we attempted to gracefully handle foreign key errors that
usually are caused by client checkins during the cleanup script's run.
This "graceful" handling had a bug that caused it to error on every
subsequent batch.

Instead, simply exit and tell the user to re-run the script.
@theforeman-bot
Copy link

Issues: #21080

@@ -32,30 +32,24 @@ namespace :katello do

existing = Katello::HostInstalledPackage.select(:installed_package_id).uniq.pluck(:installed_package_id)

interrupted_run = false
batch = 20_000
until Katello::InstalledPackage.where('id not in (?)', existing).limit(1).count == 0
begin
deleted = Katello::InstalledPackage.where('id not in (?)', existing).where("id < #{batch}").delete_all
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to also do .where("id > #{batch - 20_000}") so it would actually keep going?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dunno, I was debating it a lot before choosing to do it this way. My main concern is that if the script is too clever, it will add bugs instead of subtracting. Right now the script is very robust against an infinite loop since it's monotonic, but if an insert "snuck in" underneath the batch window's floor, we would create an infinite loop. I don't think this is possible since IDs always increase, but I suppose you could craft a setup where this happens somehow near the end of the table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I was debating whether to print the batch size and say "re-run it but use this starting point instead", and let the script take the batch starting point as an argument. It can take some time for the script to run, but I think it's better to just re-run from the beginning unless the user has a slow machine.

Copy link
Member

Choose a reason for hiding this comment

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

ah yeah that makes sense, ACK

@jlsherrill jlsherrill merged commit 0fc161c into Katello:master Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants