Merge pull request #22594 from kbrock/precache_rollup_values

Precache rollup values

(cherry picked from commit f28d107)

app/models/metric/ci_mixin/state_finders.rb

@@ -32,11 +32,20 @@ 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
 
-    vim_performance_state_for_ts(ts).public_send(assoc)
+    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)
   end
 end

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)
+    recs = obj.vim_performance_state_association(timestamp, assoc).to_a
 
     result = {}
     counts = {}

app/models/vim_performance_state.rb

@@ -111,42 +111,42 @@ def host_count_total
 
   def storages
     ids = get_assoc(:storages, :on)
-    ids.empty? ? [] : Storage.where(:id => ids).order(:id).to_a
+    ids.empty? ? Storage.none : Storage.where(:id => ids).order(:id)
   end
 
   def ext_management_systems
     ids = get_assoc(:ext_management_systems, :on)
-    ids.empty? ? [] : ExtManagementSystem.where(:id => ids).order(:id).to_a
+    ids.empty? ? ExtManagementSystem.none : ExtManagementSystem.where(:id => ids).order(:id)
   end
 
   def ems_clusters
     ids = get_assoc(:ems_clusters, :on)
-    ids.empty? ? [] : EmsCluster.where(:id => ids).order(:id).to_a
+    ids.empty? ? EmsCluster.none : EmsCluster.where(:id => ids).order(:id)
   end
 
   def hosts
     ids = get_assoc(:hosts)
-    ids.empty? ? [] : Host.where(:id => ids).order(:id).to_a
+    ids.empty? ? Host.none : Host.where(:id => ids).order(:id)
   end
 
   def vms
     ids = get_assoc(:vms)
-    ids.empty? ? [] : VmOrTemplate.where(:id => ids).order(:id).to_a
+    ids.empty? ? VmOrTemplate.none : VmOrTemplate.where(:id => ids).order(:id)
   end
 
   def container_nodes
     ids = get_assoc(:container_nodes)
-    ids.empty? ? [] : ContainerNode.where(:id => ids).order(:id).to_a
+    ids.empty? ? ContainerNode.none : ContainerNode.where(:id => ids).order(:id)
   end
 
   def container_groups
     ids = get_assoc(:container_groups)
-    ids.empty? ? [] : ContainerGroup.where(:id => ids).order(:id).to_a
+    ids.empty? ? ContainerGroup.none : ContainerGroup.where(:id => ids).order(:id)
   end
 
   def containers
     ids = get_assoc(:containers)
-    ids.empty? ? [] : Container.where(:id => ids).order(:id).to_a
+    ids.empty? ? Container.none : Container.where(:id => ids).order(:id)
   end
 
   def all_container_groups

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)
+    children = parent_perf.resource.vim_performance_state_association(ts, assoc).to_a
     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])

lib/extensions/ar_preloader.rb

@@ -0,0 +1,18 @@
+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)

lib/miq_preloader.rb

@@ -1,4 +1,26 @@
 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<ActiveRecord::Base>] records
   def self.preload(records, associations, preload_scope = nil)
     preloader = ActiveRecord::Associations::Preloader.new
     preloader.preload(records, associations, preload_scope)
@@ -10,12 +32,15 @@ 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<ActiveRecord::Base>, Object, Array<Object>]
+  # @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<ActiveRecord::Base>, Object, Array<Object>]
-  # @param association [String] an association on records
+  # @param association_name [Symbol] Name of the association
   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

spec/lib/miq_preloader_spec.rb

@@ -4,22 +4,75 @@
       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) }.to_not make_database_queries
+      expect { preload(ems, :vms) }.not_to make_database_queries
+      expect { ems.vms.size }.not_to make_database_queries
     end
 
-    it "preloads from an array" do
+    it "preloads an array" do
       emses = FactoryBot.create_list(:ems_infra, 2)
       expect { preload(emses, :vms) }.to make_database_queries(:count => 1)
-      expect(emses[0].vms).to be_loaded
+
+      expect(emses.first.vms).to be_loaded
+      expect { emses.first.vms.size }.not_to make_database_queries
     end
 
-    it "preloads from an association" do
+    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
-      expect { preload(emses, :vms) }.to make_database_queries(:count => 2)
+      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
+    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
+      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
     end
 
     def preload(*args)
@@ -34,7 +87,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) }.to_not make_database_queries
+      expect { expect(vms.size).to eq(2) }.not_to make_database_queries
     end
 
     it "preloads from an association" do
@@ -56,7 +109,7 @@ def preload_and_map(*args)
       FactoryBot.create_list(:vm, 2, :ext_management_system => ems)
 
       vms = nil
-      expect { vms = preload_and_scope(ems, :vms) }.to_not make_database_queries
+      expect { vms = preload_and_scope(ems, :vms) }.not_to make_database_queries
       expect { expect(vms.count).to eq(2) }.to make_database_queries(:count => 1)
     end
 
@@ -65,7 +118,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) }.to_not make_database_queries
+      expect { vms = preload_and_scope(ExtManagementSystem.all, :vms_and_templates) }.not_to make_database_queries
       expect { expect(vms.count).to eq(3) }.to make_database_queries(:count => 1)
     end
 
@@ -86,8 +139,8 @@ def preload_and_map(*args)
 
       hosts = nil
       vms = nil
-      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 { 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 { expect(hosts.count).to eq(2) }.to make_database_queries(:count => 1)
     end
 
@@ -103,8 +156,8 @@ def preload_and_map(*args)
 
       emses = nil
       vms = nil
-      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 { 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 { expect(vms.count).to eq(3) }.to make_database_queries(:count => 1)
     end