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 #36943 - reduce the amount of queries to get the errata counts #10810

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

sbernhard
Copy link
Member

@sbernhard sbernhard commented Nov 24, 2023

Loading the all hosts page (http://myforeman/hosts) with about 72 hosts.

OLD
Completed 200 OK in 9900ms (Views: 8613.5ms | ActiveRecord: 787.6ms | Allocations: 5520271)
Completed 200 OK in 9595ms (Views: 8381.8ms | ActiveRecord: 789.9ms | Allocations: 5524887)
SQL Queries: 195

irb(main):049:0> total=0; 100.times do x=Benchmark.measure { Host.all.each do |h| h.content_facet.nil? || h.content_facet.errata_counts_old end }; total = total + x.total; sleep 0.5; end; puts total;
68.09449500000002

NEW
Completed 200 OK in 10789ms (Views: 9794.5ms | ActiveRecord: 549.5ms | Allocations: 5511210)
Completed 200 OK in 9244ms (Views: 8400.2ms | ActiveRecord: 385.6ms | Allocations: 5511208)
SQL Queries: 65

irb(main):050:0> total=0; 100.times do x=Benchmark.measure { Host.all.each do |h| h.content_facet.nil? || h.content_facet.errata_counts end }; total = total + x.total; sleep 0.5; end; puts total;
37.18352599999951

@sbernhard
Copy link
Member Author

foreman rake benchmark for about 80 hosts.

Old

irb(main):003:0> total=0; 100.times do x=Benchmark.measure { Host.all.each do |h| h.content_facet.nil? || h.content_facet.errata_counts_old; end }; total = total + x.total; end
=> 100
irb(main):004:0> puts total
37.80042699999998

New:

irb(main):001:0> total=0; 100.times do x=Benchmark.measure { Host.all.each do |h| h.content_facet.nil? || h.content_facet.errata_counts; end }; total = total + x.total; end
=> 100
irb(main):002:0> puts total
22.924028999999997

Copy link
Member

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Looks like bugfix and security got swapped

:security => applicable_errata.security.count,
:bugfix => applicable_errata.bugfix.count,
:enhancement => applicable_errata.enhancement.count
:bugfix => applicable_errata_counts.values_at(*Katello::Erratum::SECURITY).compact.sum,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:bugfix => applicable_errata_counts.values_at(*Katello::Erratum::SECURITY).compact.sum,
:security => applicable_errata_counts.values_at(*Katello::Erratum::SECURITY).compact.sum,

@ianballou
Copy link
Member

[test katello]

Copy link
Member

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Code looks good, just need the tests to pass.

@ekohl
Copy link
Member

ekohl commented Jan 9, 2024

@ianballou good eye! I missed that. Tests are now green.

@ianballou ianballou merged commit 96da01c into Katello:master Jan 9, 2024
5 checks passed
@maximiliankolb maximiliankolb deleted the fix_36943 branch September 11, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants