Merge pull request #22604 from kbrock/precache_rollup_values

Precache rollup values

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

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:

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

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