[FINE] Faster report result index pages #4208
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rtResult https://bugzilla.redhat.com/show_bug.cgi?id=1594387 Gaprindashvili Backport commit from: ManageIQ#4143 Original SHA: ff7ee24 Rest of original message below... * * * These are new methods added in: ManageIQ/manageiq#17590 And are far more efficient for the use cases of ChargebackController and ReportController::SavedReports.
https://bugzilla.redhat.com/show_bug.cgi?id=1594387 Gaprindashvili Backport commit from: ManageIQ#4143 Original SHA: cb118f0 Rest of original message below... * * * Calls to `.length` will fetch the entire record set or rows for the MiqReportResult, and in the current case of these button helpers, throw them all away without using any of the data fetched. In most cases, the `html_details` relation is probably what was is being used, so even caching these ahead of time is wasteful. By using `.exists?` as an alternative, we basically do a: SELECT 1 as one FROM miq_report_result_details WHERE "miq_report_result_details"."miq_report_result_id" = ? LIMIT 1 Which only returns a single digit from the database if it has at least one record and nothing if it doesn't. This will also be cached in a controller action by the ActiveRecord query cache, so it will also require zero queries the second time around.
…ng mixin https://bugzilla.redhat.com/show_bug.cgi?id=1594387 Gaprindashvili Backport commit from: ManageIQ#4143 Original SHA: b5260fd Rest of original message below... * * * This not only makes it so we are consistent between the main controller and the mixin, but this is also much faster to use this over `MiqReportResult#report_results` when there is a large `binary_blob` associated with the `MiqReportResult` record.
|
Some comments on commits NickLaMuro/manageiq-ui-classic@64f8b1f~...02fd904 spec/controllers/miq_report_controller/trees_spec.rb
|
|
Checked commits NickLaMuro/manageiq-ui-classic@64f8b1f~...02fd904 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 app/controllers/chargeback_controller.rb
app/controllers/report_controller/saved_reports.rb
|
mzazrivec
approved these changes
Jun 26, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the Fine port of #4143. Original PR message below.
Merge after ManageIQ/manageiq#17590 (NOTE: Not relavent, this is already backported)
This makes both the
ChargebackControllerandReportController::SavedReportsactions for viewing reports signficantly faster by:binary_blobdata entirely (avoids massive YAML deserialization). This uses methods provided by Add faster MiqReportResult helper methods for viewing saved report results manageiq#17590 to accomplish this..exists?instead of.length > 0, which prevents needing to fetch all of the report result row records, which were then thrown away and never used.Benchmarks
This is displaying a
MiqReportResultindex page that has 23k+ rows associated with it. Showing the first page, paginated by 20 records.Before
After
Links
Merge First!Already done): Add faster MiqReportResult helper methods for viewing saved report results manageiq#17590QA Steps
I tested the did the following when running the
Before/Afterbenchmarks:adminand go toCloud Intel->ReportsDownloadtoolbar button as well.