Katello / katello-packaging

[DEPRECATED] Packaging for Katello
7 stars 33 forks source link

Fixes #19941 - make katello-remove remove all RPMs in one go #464

Closed evgeni closed 7 years ago

evgeni commented 7 years ago

A small benchmark on CentOS7 with 272 packages to be removed:

without the patch:

real    9m0.638s
user    4m8.864s
sys     0m31.247s

with the patch:

real    4m32.901s
user    1m14.804s
sys     0m15.476s
evgeni commented 7 years ago

Broken Koji is broken :'(

ehelms commented 7 years ago

So the hope here is that there is no one package in the mix that breaks the remove or if there is users have to stop and figure out how to solve the issue? What if a package being removed is being used by something on the box that the user has added and wants to keep around? Do we tell them to run this command manually removing the one or two packages from it?

evgeni commented 7 years ago

No, we currently just tell the user that a whole bunch of packages is to be removed, and ask for confirmation (but the list we ask is not really complete, tbh). After that we just run the command. Previously in a loop, now just one go. So there is no behavior change as such present.

Also the old and the new code don't check if yum was successful removing the packages.

ehelms commented 7 years ago

Right, but in the previous iteration it would try and delete each package and only those that failed would be left around correct? This does change the behavior to fail out of removing any packages if one fails?

cfouant commented 7 years ago

@ehelms - Ah, so if one package fails, the entire command fails?

sean797 commented 7 years ago

@ehelms This doesn't change the behavior. If any fail yum will print a list of Failed packages just like it will print a list of Installed ones after every transaction. https://github.com/rpm-software-management/yum/blob/master/output.py#L1734-L1742

Example of Installed output below, I cannot find any Failed output online. Image

EDIT: This is actually exactly how it use to work before it was re-written in Ruby. https://github.com/Katello/katello-packaging/blob/e1a51759744f7826c9a02037551a51227b884b9a/katello/katello-remove#L166

jlsherrill commented 7 years ago

[test]

ehelms commented 7 years ago

Fair enough - I just to make sure that troubleshooting was not being sacrificed for speed in an operation that should not be performed that often to warrant optimizations in lieu of the ability to.