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 #25173 - Fix slow enabled report generation #78

Closed
wants to merge 7 commits into from

Conversation

kevin-bowers
Copy link
Contributor

In its current state, this piece of the enabled_repos_upload yum/dnf plugin imports the yum or dnf Python module and creates an associated object each time _replace_vars() is executed. This can lead to time consuming yum operations if there are a large number of enabled yum repositories.

Example

A server has 13 enabled yum repositories. With yum.YumBase() being re-executed with each run of _replace_vars(). This results in the following output from a simple yum check-update:

# time yum check-update
Loaded plugins: enabled_repos_upload, fastestmirror, package_upload, product-id, search-disabled-repos, subscription-manager, tracer_upload
Loading mirror speeds from cached hostfile
Centos_Centos_7_Base
Centos_Centos_7_Extras
Centos_Centos_7_Updates
Centos_Centos_sclo
Centos_Centos_sclo-rh
Datadog_Datadog
EPEL_EPEL_Centos7
Extra_rpm-extra
Katello-agent_agent-3_7
MySQL_Enterprise_MySQL_Enterprise
Puppet_puppet_4
Zabbix_Zabbix_3_2
Uploading Enabled Repositories Report
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager
Loaded plugins: fastestmirror, product-id, subscription-manager

real	0m35.492s
user	0m8.330s
sys	0m0.611s

The output is excessively long and it takes 35 seconds to tell me that nothing is available to update. With this PR in place, it only takes 6 seconds and the output is much shorter:

# time yum check-update
Loaded plugins: enabled_repos_upload, fastestmirror, package_upload, product-id, search-disabled-repos, subscription-manager, tracer_upload
Loading mirror speeds from cached hostfile
Centos_Centos_7_Base
Centos_Centos_7_Extras
Centos_Centos_7_Updates
Centos_Centos_sclo
Centos_Centos_sclo-rh
Datadog_Datadog
EPEL_EPEL_Centos7
Extra_rpm-extra
Katello-agent_agent-3_7
MySQL_Enterprise_MySQL_Enterprise
Puppet_puppet_4
Zabbix_Zabbix_3_2
Uploading Enabled Repositories Report
Loaded plugins: fastestmirror, product-id, subscription-manager

real	0m6.351s
user	0m3.058s
sys	0m0.417s

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 1913674 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@kevin-bowers kevin-bowers changed the title Fixes #25173 - Fix slow enabled report generation fixes #25173 - Fix slow enabled report generation Oct 11, 2018
@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 1913674 must be in the format fixes #redmine_number - brief description
  • 93646de must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@jturel
Copy link
Member

jturel commented Oct 11, 2018

@kevin-bowers thanks for the PR and the thorough analysis. 💯

I currently have another work-in-progress PR (#77) open that will affect this area of the code. Once that is merged (in the next few days hopefully) I'll ask you to rebase this PR.

@jturel
Copy link
Member

jturel commented Oct 12, 2018

@kevin-bowers you can go ahead and rebase - the issue should still be present so your fix is still valid. After that I'll do some testing of my own :)

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 1913674 must be in the format fixes #redmine_number - brief description
  • 93646de must be in the format fixes #redmine_number - brief description
  • de0078b must be in the format fixes #redmine_number - brief description
  • 41c7210 must be in the format fixes #redmine_number - brief description
  • b6d034c must be in the format fixes #redmine_number - brief description
  • 685dc63 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@kevin-bowers
Copy link
Contributor Author

Just re-based. I like your changes too. I applied the same fix from before.

@jturel
Copy link
Member

jturel commented Oct 15, 2018

It looks like your rebase didn't go quite right. Looking at the 'Commits' tab I see there are 7 commits - one of them in the middle is my recent commit that you wanted to base on top of! I'd expect to see a single commit on this PR with the commit message in the same format as the name of this PR: 'fixes #.....'

Might be a good use of interactive rebase from your working copy: git rebase -i origin/master and you can re-jigger the commits around (or remove outdated ones) to make it work. You might still run into some merge conflicts but it's an effective way to reconcile your branch. Let me know if I can help!

@kevin-bowers
Copy link
Contributor Author

It looks like I have totally screwed up my branch. Not sure how I did that. I'm going to close this, re-branch, and open a new PR. Sorry about that!

@kevin-bowers kevin-bowers deleted the dnf-yum-fix branch October 15, 2018 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants