Skip to content

Commit

Permalink
Avoid duplicate host load in allowed_hosts_obj
Browse files Browse the repository at this point in the history
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 |
  • Loading branch information
NickLaMuro committed May 9, 2018
1 parent c2e01c9 commit 809f655
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion app/models/miq_request_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ def allowed_hosts_obj(options = {})
return [] if hosts_ids.blank?

# Remove any hosts that are no longer in the list
all_hosts = load_ar_obj(src[:ems]).hosts.find_all { |h| hosts_ids.include?(h.id) }
all_hosts = load_ar_obj(src[:ems]).hosts.where(:id => hosts_ids)
allowed_hosts_obj_cache = process_filter(:host_filter, Host, all_hosts)
_log.info("allowed_hosts_obj returned [#{allowed_hosts_obj_cache.length}] objects in [#{Time.now - st}] seconds")
rails_logger('allowed_hosts_obj', 1)
Expand Down

0 comments on commit 809f655

Please sign in to comment.