Skip to content

Commit

Permalink
Cache vim_performance_state relations
Browse files Browse the repository at this point in the history
Fixed bug: It was only loading a single association

The MiqPreloader.preload was not properly loading

we were having trouble that the reverse relation (assoc.parent) was not being cached
Ended up deciding caching the forward and the reverse as the best

For running hourly, we do run the realtime first, so make sure this works
for running multiple relations for multiple timestamps

Had thought a simple association(vps_assoc)&.reset would work, but it threw errors
if we looked up an invalid association, so had to filter them with the set logic
  • Loading branch information
kbrock committed Jul 12, 2023
1 parent 559df70 commit 74379ef
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 12 deletions.
18 changes: 14 additions & 4 deletions app/models/metric/ci_mixin/state_finders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
72 changes: 64 additions & 8 deletions spec/models/metric/ci_mixin/state_finders_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -114,16 +171,15 @@

private

def create_vps(image, timestamp, containers = [], nodes = [])
def create_vps(parent, timestamp, association = {})
FactoryBot.create(
:vim_performance_state,
:resource => image,
: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
Expand Down

0 comments on commit 74379ef

Please sign in to comment.