diff --git a/app/models/metric/ci_mixin/state_finders.rb b/app/models/metric/ci_mixin/state_finders.rb index 670708e31f2..f5638826107 100644 --- a/app/models/metric/ci_mixin/state_finders.rb +++ b/app/models/metric/ci_mixin/state_finders.rb @@ -36,16 +36,26 @@ def preload_vim_performance_state_for_ts_iso8601(conditions = {}) # 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 : [] + return respond_to?(:miq_regions) ? miq_regions : MiqRegion.none + end + + # this is a virtual reflection, just return the value + if !self.class.reflect_on_association(assoc) + return vim_performance_state_for_ts(ts).public_send(assoc) 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) + # we are using a different timestamp + # clear out relevant associations + (VimPerformanceState::ASSOCIATIONS & self.class.reflections.keys.map(&:to_sym)).each do |vps_assoc| + association(vps_assoc).reset + end end + if !association(assoc.to_sym).loaded? + MiqPreloader.preload_from_array(self, assoc, vim_performance_state_for_ts(ts).public_send(assoc)) + end public_send(assoc) end end diff --git a/lib/miq_preloader.rb b/lib/miq_preloader.rb index 4e1fde98f1a..71b260d426c 100644 --- a/lib/miq_preloader.rb +++ b/lib/miq_preloader.rb @@ -26,6 +26,15 @@ def self.preload(records, associations, preload_scope = nil) preloader.preload(records, associations, preload_scope) end + # for a record, cache results. Also cache the children's links back + # currently preload works for simple associations, but this is needed for reverse associations + def self.preload_from_array(record, association_name, values) + association = record.association(association_name.to_sym) + values = Array.wrap(values) + association.target = association.reflection.collection? ? values : values.first + values.each { |value| association.set_inverse_instance(value) } + end + # it will load records and their associations, and return the children # # instead of N+1: diff --git a/spec/lib/miq_preloader_spec.rb b/spec/lib/miq_preloader_spec.rb index 0893e5b4c6b..9d22a8d9a1a 100644 --- a/spec/lib/miq_preloader_spec.rb +++ b/spec/lib/miq_preloader_spec.rb @@ -6,7 +6,7 @@ let(:image) do FactoryBot.create(:container_image).tap do |image| FactoryBot.create( - :container, + :container, :container_image => image, :container_group => FactoryBot.create(:container_group, :container_node => FactoryBot.create(:container_node)) ) @@ -114,6 +114,62 @@ def preload(*args) end end + describe ".preload_from_array" do + it "preloads a loaded simple relation" do + vms = Vm.where(:ems_id => ems.id).load + + expect { preload_from_array(ems, :vms, vms) }.not_to make_database_queries + expect { preload_from_array(ems, :vms, vms) }.not_to make_database_queries + expect { expect(ems.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 an unloaded simple relation" do + vms = Vm.where(:ems_id => ems.id) + vms2 = vms.order(:id).to_a # preloaded vms to be used in tests + + expect { preload_from_array(ems, :vms, vms) }.to make_database_queries(:count => 1) + expect { preload_from_array(ems, :vms, vms) }.not_to make_database_queries + expect { expect(ems.vms).to match_array(vms2) }.not_to make_database_queries + expect { expect(vms.first.ext_management_system).to eq(ems) }.not_to make_database_queries + end + + it "preloads an array of a simple relation" do + vms = Vm.where(:ems_id => ems.id).to_a + + expect { preload_from_array(ems, :vms, vms) }.not_to make_database_queries + expect { preload_from_array(ems, :vms, vms) }.not_to make_database_queries + expect { expect(ems.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 a loaded through relation" do + image + nodes = ContainerNode.all.load + + expect { preload_from_array(image, :container_nodes, nodes) }.not_to make_database_queries + expect { preload_from_array(image, :container_nodes, nodes) }.not_to make_database_queries + expect { expect(image.container_nodes).to eq(nodes) }.not_to make_database_queries + # TODO: can we get this to 0 queries? + expect { expect(nodes.first.container_images).to eq([image]) }.to make_database_queries(:count => 1) + end + + it "preloads an array of a through relation" do + image + nodes = ContainerNode.all.to_a + + expect { preload_from_array(image, :container_nodes, nodes) }.not_to make_database_queries + expect { preload_from_array(image, :container_nodes, nodes) }.not_to make_database_queries + expect { expect(image.container_nodes).to eq(nodes) }.not_to make_database_queries + # TODO: can we get this to 0 queries? + expect { expect(nodes.first.container_images).to eq([image]) }.to make_database_queries(:count => 1) + end + + def preload_from_array(record, relation, values) + MiqPreloader.preload_from_array(record, relation, values) + end + end + describe ".preload_and_map" do it "preloads from an object" do ems = FactoryBot.create(:ems_infra) @@ -166,10 +222,10 @@ def preload_and_map(*args) it "preloads (object.all).has_many.belongs_to" do ems = FactoryBot.create(:ems_infra) FactoryBot.create_list(:vm, 2, - :ext_management_system => ems, - :host => FactoryBot.create(:host, :ext_management_system => ems)) + :ext_management_system => ems, + :host => FactoryBot.create(:host, :ext_management_system => ems)) FactoryBot.create(:vm, :ext_management_system => ems, - :host => FactoryBot.create(:host, :ext_management_system => ems)) + :host => FactoryBot.create(:host, :ext_management_system => ems)) hosts = nil vms = nil diff --git a/spec/models/metric/ci_mixin/state_finders_spec.rb b/spec/models/metric/ci_mixin/state_finders_spec.rb index c1574a4a201..a4ed1334b16 100644 --- a/spec/models/metric/ci_mixin/state_finders_spec.rb +++ b/spec/models/metric/ci_mixin/state_finders_spec.rb @@ -2,12 +2,69 @@ let(:image) { FactoryBot.create(:container_image) } let(:container1) { FactoryBot.create(:container, :container_image => image) } let(:container2) { FactoryBot.create(:container, :container_image => image) } - let(:node1) { FactoryBot.create(:container_node) } - let(:node2) { FactoryBot.create(:container_node) } + + # region is currently the only class that has multiple rollups + let(:region) { MiqRegion.my_region || MiqRegion.seed } + let(:ems1) { FactoryBot.create(:ext_management_system) } # implied :region => region + let(:ems2) { FactoryBot.create(:ext_management_system) } + let(:storage1) { FactoryBot.create(:storage) } + let(:storage2) { FactoryBot.create(:storage) } let(:ts_now) { Time.now.utc.beginning_of_hour.to_s } let(:timestamp) { 2.hours.ago.utc.beginning_of_hour.to_s } + describe "#vim_performance_state_association" do + let(:c_vps_now) { create_vps(image, ts_now, :containers => [container1, container2]) } + let(:c_vps) { create_vps(image, timestamp, :containers => [container1]) } + + let(:r_vps_now) { create_vps(region, ts_now, :ext_management_systems => [ems1, ems2], :storages => [storage1, storage2]) } + let(:r_vps) { create_vps(region, timestamp, :ext_management_systems => [ems1], :storages => [storage1]) } + + it "performs a single query when looking up an association multiple times" do + c_vps + + expect do + expect(image.vim_performance_state_association(timestamp, :containers)).to eq([container1]) + end.to make_database_queries(:count => 2) + + expect do + expect(image.vim_performance_state_association(timestamp, :containers)).to eq([container1]) + end.to make_database_queries(:count => 0) + end + + it "supports virtual associations" do + r_vps + + expect do + expect(region.vim_performance_state_association(timestamp, :ext_management_systems)).to eq([ems1]) + end.to make_database_queries(:count => 2) + + expect do + expect(region.vim_performance_state_association(timestamp, :ext_management_systems)).to eq([ems1]) + end.to make_database_queries(:count => 2) # TODO: vps caching (another PR) will change to 1 + end + + it "fetches a second timestamp" do + c_vps + c_vps_now + expect(image.vim_performance_state_association(timestamp, :containers)).to match_array([container1]) + + expect do + expect(image.vim_performance_state_association(ts_now, :containers)).to match_array([container1, container2]) + end.to make_database_queries(:count => 2) + end + + it "assigns reverse association" do + c_vps + expect(image.vim_performance_state_association(timestamp, :containers)).to match_array([container1]) + + expect do + c = image.vim_performance_state_association(timestamp, :containers).first + expect(c.container_image).to eq(image) + end.to make_database_queries(:count => 0) + end + end + # NOTE: in these specs, we could let perf_capture_state be called # but using this reduces the queries describe "#vim_performance_state_for_ts" do @@ -114,16 +171,15 @@ private - def create_vps(image, ts, containers = [], nodes = []) + def create_vps(parent, timestamp, association = {}) FactoryBot.create( :vim_performance_state, - :resource => image, - :timestamp => ts, + :resource => parent, + :timestamp => timestamp, :state_data => { - :assoc_ids => { - :containers => {:on => containers.map(&:id), :off => []}, - :container_nodes => {:on => nodes.map(&:id), :off => []}, - } + :assoc_ids => association.transform_values do |values| + {:on => values.map(&:id), :off => []} + end } ) end