From 809f655e63f76204083b9f115763a70c9a6f0b54 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 9 May 2018 15:54:35 -0500 Subject: [PATCH] Avoid duplicate host load in allowed_hosts_obj 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 | --- app/models/miq_request_workflow.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/miq_request_workflow.rb b/app/models/miq_request_workflow.rb index f6207b56db2..202731cee59 100644 --- a/app/models/miq_request_workflow.rb +++ b/app/models/miq_request_workflow.rb @@ -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)