From bd6431cc17d0c5e72c9ad7de0c2455c9cbb63e84 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Fri, 7 Jul 2023 09:47:48 -0400 Subject: [PATCH] Revert "Merge pull request #22594 from kbrock/precache_rollup_values" This reverts commit f28d10779b15316e27578175c07145a3b35469bf, reversing changes made to 4b5fe6070fff1f9eb4d51283f3b0ccfa786628e4. --- app/models/metric/ci_mixin/state_finders.rb | 11 +-- app/models/metric/rollup.rb | 2 +- app/models/vim_performance_state.rb | 16 ++--- app/models/vim_performance_tag_value.rb | 2 +- lib/extensions/ar_preloader.rb | 18 ----- lib/miq_preloader.rb | 27 +------ spec/lib/miq_preloader_spec.rb | 79 ++++----------------- 7 files changed, 25 insertions(+), 130 deletions(-) delete mode 100644 lib/extensions/ar_preloader.rb diff --git a/app/models/metric/ci_mixin/state_finders.rb b/app/models/metric/ci_mixin/state_finders.rb index 670708e31f28..7fa2c2cb0131 100644 --- a/app/models/metric/ci_mixin/state_finders.rb +++ b/app/models/metric/ci_mixin/state_finders.rb @@ -32,20 +32,11 @@ def preload_vim_performance_state_for_ts_iso8601(conditions = {}) @states_by_ts = vim_performance_states.where(conditions).index_by { |x| x.timestamp.utc.iso8601 } end - # Use VimPerformanceState to populate the scopes with the values from a particular point in time - # using @last_vps_ts to ensure we don't load at one time and then use at another def vim_performance_state_association(ts, assoc) if assoc.to_s == "miq_regions" return respond_to?(:miq_regions) ? miq_regions : [] end - if !defined?(@last_vps_ts) || @last_vps_ts != ts - @last_vps_ts = ts - public_send(assoc).reset - - MiqPreloader.preload(self, assoc, vim_performance_state_for_ts(ts).public_send(assoc).load) - end - - public_send(assoc) + vim_performance_state_for_ts(ts).public_send(assoc) end end diff --git a/app/models/metric/rollup.rb b/app/models/metric/rollup.rb index aa433fe0da47..7112f66b42e3 100644 --- a/app/models/metric/rollup.rb +++ b/app/models/metric/rollup.rb @@ -268,7 +268,7 @@ def self.rollup_realtime_perfs(obj, rt_perfs, new_perf = {}) def self.rollup_child_metrics(obj, timestamp, interval_name, assoc) ts = timestamp.kind_of?(Time) ? timestamp.utc.iso8601 : timestamp - recs = obj.vim_performance_state_association(timestamp, assoc).to_a + recs = obj.vim_performance_state_association(timestamp, assoc) result = {} counts = {} diff --git a/app/models/vim_performance_state.rb b/app/models/vim_performance_state.rb index b329c64768b6..59198152f4e2 100644 --- a/app/models/vim_performance_state.rb +++ b/app/models/vim_performance_state.rb @@ -111,42 +111,42 @@ def host_count_total def storages ids = get_assoc(:storages, :on) - ids.empty? ? Storage.none : Storage.where(:id => ids).order(:id) + ids.empty? ? [] : Storage.where(:id => ids).order(:id).to_a end def ext_management_systems ids = get_assoc(:ext_management_systems, :on) - ids.empty? ? ExtManagementSystem.none : ExtManagementSystem.where(:id => ids).order(:id) + ids.empty? ? [] : ExtManagementSystem.where(:id => ids).order(:id).to_a end def ems_clusters ids = get_assoc(:ems_clusters, :on) - ids.empty? ? EmsCluster.none : EmsCluster.where(:id => ids).order(:id) + ids.empty? ? [] : EmsCluster.where(:id => ids).order(:id).to_a end def hosts ids = get_assoc(:hosts) - ids.empty? ? Host.none : Host.where(:id => ids).order(:id) + ids.empty? ? [] : Host.where(:id => ids).order(:id).to_a end def vms ids = get_assoc(:vms) - ids.empty? ? VmOrTemplate.none : VmOrTemplate.where(:id => ids).order(:id) + ids.empty? ? [] : VmOrTemplate.where(:id => ids).order(:id).to_a end def container_nodes ids = get_assoc(:container_nodes) - ids.empty? ? ContainerNode.none : ContainerNode.where(:id => ids).order(:id) + ids.empty? ? [] : ContainerNode.where(:id => ids).order(:id).to_a end def container_groups ids = get_assoc(:container_groups) - ids.empty? ? ContainerGroup.none : ContainerGroup.where(:id => ids).order(:id) + ids.empty? ? [] : ContainerGroup.where(:id => ids).order(:id).to_a end def containers ids = get_assoc(:containers) - ids.empty? ? Container.none : Container.where(:id => ids).order(:id) + ids.empty? ? [] : Container.where(:id => ids).order(:id).to_a end def all_container_groups diff --git a/app/models/vim_performance_tag_value.rb b/app/models/vim_performance_tag_value.rb index cae3d38faac3..0688cd25eb65 100644 --- a/app/models/vim_performance_tag_value.rb +++ b/app/models/vim_performance_tag_value.rb @@ -56,7 +56,7 @@ def self.build_for_association(parent_perf, assoc, options = {}) return [] if eligible_cats.empty? ts = parent_perf.timestamp - children = parent_perf.resource.vim_performance_state_association(ts, assoc).to_a + children = parent_perf.resource.vim_performance_state_association(ts, assoc) return [] if children.empty? vim_performance_daily = parent_perf.kind_of?(VimPerformanceDaily) recs = get_metrics(children, ts, parent_perf.capture_interval_name, vim_performance_daily, options[:category]) diff --git a/lib/extensions/ar_preloader.rb b/lib/extensions/ar_preloader.rb deleted file mode 100644 index d1b23f44f82b..000000000000 --- a/lib/extensions/ar_preloader.rb +++ /dev/null @@ -1,18 +0,0 @@ -module ActiveRecordPreloadScopes - # based upon active record 6.1 - def records_for(ids) - # use our logic if passing in [ActiveRecord::Base] or passing in a loaded Relation/scope - unless (preload_scope.kind_of?(Array) && preload_scope.first.kind_of?(ActiveRecord::Base)) || - preload_scope.try(:loaded?) - return super - end - - preload_scope.each do |record| - owner = owners_by_key[convert_key(record[association_key_name])].first - association = owner.association(reflection.name) - association.set_inverse_instance(record) - end - end -end - -ActiveRecord::Associations::Preloader::Association.prepend(ActiveRecordPreloadScopes) diff --git a/lib/miq_preloader.rb b/lib/miq_preloader.rb index 4e1fde98f1af..6b3bcb1f4eb1 100644 --- a/lib/miq_preloader.rb +++ b/lib/miq_preloader.rb @@ -1,26 +1,4 @@ module MiqPreloader - # If you want to preload an association on multiple records - # or want to only load a subset of an association - # - # @example Preloading vms on a set of emses - # vms_scope = Vm.where(:ems_id => emses.id) - # preload(emses, :vms, vms_scope) - # emses.map { |ems| ems.vms } # cached - no queries - # vms_scope.first.ems # cached - the reversed association is cached - # - # @example Programmatically determine the reverse association name - # Going from Ems#association(:vms) and going to Vm#association(:ems) - # - # reverse_association_name = record.class.reflect_on_association(association).inverse_of.name - # reverse_association = result.association(reverse_association_name) - # - # @param record [relation|ActiveRecord::Base|Array[ActiveRecord::Base]] - # @param association [Symbol|Hash|Array] name of the association(s) - # @param preload_scope [Nil|relation] Relation of the records to be use for preloading - # For all but one case, default behavior is to use the association - # Alternatively a scope can be used. - # Currently an array does not work - # @return [Array] records def self.preload(records, associations, preload_scope = nil) preloader = ActiveRecord::Associations::Preloader.new preloader.preload(records, associations, preload_scope) @@ -32,15 +10,12 @@ def self.preload(records, associations, preload_scope = nil) # orchestration_stack.subtree.flat_map(&:direct_vms) # use instead: # preload_and_map(orchestration_stack.subtree, :direct_vms) - # - # @param records [ActiveRecord::Base, Array, Object, Array] - # @param association [Symbol] name of the association def self.preload_and_map(records, association) Array.wrap(records).tap { |recs| MiqPreloader.preload(recs, association) }.flat_map(&association) end # @param records [ActiveRecord::Base, Array, Object, Array] - # @param association_name [Symbol] Name of the association + # @param association [String] an association on records def self.preload_and_scope(records, association_name) records = Array.wrap(records) unless records.kind_of?(Enumerable) active_record_klass = records.respond_to?(:klass) ? records.klass : records.first.class diff --git a/spec/lib/miq_preloader_spec.rb b/spec/lib/miq_preloader_spec.rb index c66db0ad8c51..88ad890905d3 100644 --- a/spec/lib/miq_preloader_spec.rb +++ b/spec/lib/miq_preloader_spec.rb @@ -4,75 +4,22 @@ ems = FactoryBot.create(:ems_infra) expect(ems.vms).not_to be_loaded expect { preload(ems, :vms) }.to make_database_queries(:count => 1) - expect(ems.vms).to be_loaded - expect { preload(ems, :vms) }.not_to make_database_queries - expect { ems.vms.size }.not_to make_database_queries + expect { preload(ems, :vms) }.to_not make_database_queries end - it "preloads an array" do + it "preloads from an array" do emses = FactoryBot.create_list(:ems_infra, 2) expect { preload(emses, :vms) }.to make_database_queries(:count => 1) - - expect(emses.first.vms).to be_loaded - expect { emses.first.vms.size }.not_to make_database_queries - end - - it "preloads a relation (record is a relation)" do - FactoryBot.create_list(:vm, 2, :ext_management_system => FactoryBot.create(:ems_infra)) - emses = ExtManagementSystem.all - expect { preload(emses, :vms) }.to make_database_queries(:count => 2) - - expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries - end - - it "preloads with a relation (records is a relation)" do - ems = FactoryBot.create(:ems_infra) - FactoryBot.create_list(:vm, 2, :ext_management_system => ems) - - emses = ExtManagementSystem.all.load - vms = Vm.where(:ems_id => emses.select(:id)) - - expect { preload(emses, :vms, vms) }.to make_database_queries(:count => 1) - end - - it "preloads with a loaded relation (records is a relation)" do - ems = FactoryBot.create(:ems_infra) - FactoryBot.create_list(:vm, 2, :ext_management_system => ems) - - emses = ExtManagementSystem.all.load - vms = Vm.where(:ems_id => emses.select(:id)).load - - expect { preload(emses, :vms, vms) }.not_to make_database_queries - expect { preload(emses, :vms, vms) }.not_to make_database_queries - expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries - expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries + expect(emses[0].vms).to be_loaded end - it "preloads with an array (records is a relation)" do - ems = FactoryBot.create(:ems_infra) - FactoryBot.create_list(:vm, 2, :ext_management_system => ems) - - emses = ExtManagementSystem.all.load - vms = Vm.where(:ems_id => emses.select(:id)).to_a - - expect { preload(emses, :vms, vms) }.not_to make_database_queries - expect { preload(emses, :vms, vms) }.not_to make_database_queries - expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries - expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries - end - - it "preloads with an array (records is an array)" do + it "preloads from an association" do ems = FactoryBot.create(:ems_infra) FactoryBot.create_list(:vm, 2, :ext_management_system => ems) - emses = ExtManagementSystem.all.load.to_a - vms = Vm.where(:ems_id => emses.map(&:id)).to_a - - expect { preload(emses, :vms, vms) }.not_to make_database_queries - expect { preload(emses, :vms, vms) }.not_to make_database_queries - expect { expect(emses.first.vms.size).to eq(2) }.not_to make_database_queries - expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries + emses = ExtManagementSystem.all + expect { preload(emses, :vms) }.to make_database_queries(:count => 2) end def preload(*args) @@ -87,7 +34,7 @@ def preload(*args) vms = nil expect { vms = preload_and_map(ems, :vms) }.to make_database_queries(:count => 1) - expect { expect(vms.size).to eq(2) }.not_to make_database_queries + expect { expect(vms.size).to eq(2) }.to_not make_database_queries end it "preloads from an association" do @@ -109,7 +56,7 @@ def preload_and_map(*args) FactoryBot.create_list(:vm, 2, :ext_management_system => ems) vms = nil - expect { vms = preload_and_scope(ems, :vms) }.not_to make_database_queries + expect { vms = preload_and_scope(ems, :vms) }.to_not make_database_queries expect { expect(vms.count).to eq(2) }.to make_database_queries(:count => 1) end @@ -118,7 +65,7 @@ def preload_and_map(*args) FactoryBot.create(:template, :ext_management_system => FactoryBot.create(:ems_infra)) vms = nil - expect { vms = preload_and_scope(ExtManagementSystem.all, :vms_and_templates) }.not_to make_database_queries + expect { vms = preload_and_scope(ExtManagementSystem.all, :vms_and_templates) }.to_not make_database_queries expect { expect(vms.count).to eq(3) }.to make_database_queries(:count => 1) end @@ -139,8 +86,8 @@ def preload_and_map(*args) hosts = nil vms = nil - expect { vms = preload_and_scope(ExtManagementSystem.all, :vms_and_templates) }.not_to make_database_queries - expect { hosts = preload_and_scope(vms, :host) }.not_to make_database_queries + expect { vms = preload_and_scope(ExtManagementSystem.all, :vms_and_templates) }.to_not make_database_queries + expect { hosts = preload_and_scope(vms, :host) }.to_not make_database_queries expect { expect(hosts.count).to eq(2) }.to make_database_queries(:count => 1) end @@ -156,8 +103,8 @@ def preload_and_map(*args) emses = nil vms = nil - expect { emses = preload_and_scope(Host.all, :ext_management_system) }.not_to make_database_queries - expect { vms = preload_and_scope(emses, :vms) }.not_to make_database_queries + expect { emses = preload_and_scope(Host.all, :ext_management_system) }.to_not make_database_queries + expect { vms = preload_and_scope(emses, :vms) }.to_not make_database_queries expect { expect(vms.count).to eq(3) }.to make_database_queries(:count => 1) end