Avoid duplicate host load in allowed_hosts_obj #17402
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
MiqRequestWorkflow#allowed_hosts_obj originally would collect the known host ids from the ems tree it has already built up, combine those with any hosts associated to the storage it had, do a query against the `src[:ems]`'s hosts, and select only the ones that match the original set of host ids. From there, it would take that list of host objects, throw it through Rbac, which would also make another query to find the hosts that match that list AND filter it based on the targeted user, throwing out the previously instantiated hosts that were used as the intermediate list from `src[:ems]`. This change simply avoids creating that intermediate `Host` object list by replacing the `.find_all` with a `.where`, meaning we pass a scoped query to Rbac (instead of an `Array`) that hasn't been executed until it has the user filter applied to it. Metrics ------- Benchmarks were taken by monitoring the UI requests using the `manageiq_performance` gem. The route monitored was selecting the `"Lifecycle" -> "Publish this VM to a Template"` menu button from a VM show page. The metrics shown are against database with a fairly large EMS, with about 600 hosts, and are 3 subsequent requests against that route. **Before** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 18613 | 2062 | 1463.7 | 70017 | | 17695 | 2062 | 1475.5 | 70017 | | 17774 | 2062 | 1578.4 | 70017 | **After** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 18553 | 2061 | 1560.7 | 61866 | | 17385 | 2061 | 1564.6 | 61866 | | 17468 | 2061 | 1437.5 | 61866 |
NickLaMuro
force-pushed
the
lighter_queries_in_miq_request_workflow
branch
from
ac52ed3 to
809f655
Compare
|
Checked commit NickLaMuro@809f655 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
This was referenced May 9, 2018
Merged
d-m-u
approved these changes
May 16, 2018
|
@miq-bot add_label fine/yes, gaprindashvili/yes |
simaishi
pushed a commit
that referenced
this pull request
Jun 21, 2018
…est_workflow Avoid duplicate host load in allowed_hosts_obj (cherry picked from commit d577df7) https://bugzilla.redhat.com/show_bug.cgi?id=1593798
|
Fine backport details: |
simaishi
pushed a commit
that referenced
this pull request
Jun 21, 2018
…est_workflow Avoid duplicate host load in allowed_hosts_obj (cherry picked from commit d577df7) https://bugzilla.redhat.com/show_bug.cgi?id=1593797
|
Gaprindashvili backport details: |
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.
MiqRequestWorkflow#allowed_hosts_objoriginally would collect the known host ids from the ems tree it has already built up, combine those with any hosts associated to the storage it had, do a query against thesrc[:ems]'s hosts, and select only the ones that match the original set of host ids. From there, it would take that list of host objects, throw it through Rbac, which would also make another query to find the hosts that match that list AND filter it based on the targeted user, throwing out the previously instantiated hosts that were used as the intermediate list fromsrc[:ems].This change simply avoids creating that intermediate
Hostobject list by replacing the.find_allwith a.where, meaning we pass a scoped query to Rbac (instead of anArray) that hasn't been executed until it has the user filter applied to it.Metrics
Benchmarks were taken by monitoring the UI requests using the
manageiq_performancegem. The route monitored was selecting the"Lifecycle" -> "Publish this VM to a Template"menu button from a VM show page. The metrics shown are against database with a fairly large EMS, with about 600 hosts, and are 3 subsequent requests against that route.Note the change in
rowcounts:Before
After
Note: The benchmarks for this change do NOT include the changes from other PRs (#17354 for example). Benchmarks of all changes can be found here.
Links