Skip to content

Commit

Permalink
Don't lookup previously created VimPerformanceState
Browse files Browse the repository at this point in the history
wanted to create specs around vim_performance_state_association and hit a tangent.

This already cached newly created vim_performance_state records when one did not exist in the db
but it never looked for that value in the local cache.

now it looks that value up
  • Loading branch information
kbrock committed Jul 15, 2023
1 parent 14babf7 commit 9c541c7
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
7 changes: 5 additions & 2 deletions app/models/metric/ci_mixin/state_finders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Metric::CiMixin::StateFinders
# contains a subset, typically top of the hour and the timestamp of interest
# the cache is indexed by a String in iso8601 form
# used by: CiMixin::Processing#perf_process
# @param ts [Time|String] prefer Time
# @param ts [Time|String] beginning of hour timestamp (prefer Time)
def vim_performance_state_for_ts(ts)
ts = Time.parse(ts).utc if ts.kind_of?(String)
ts_iso = ts.utc.iso8601
Expand All @@ -25,9 +25,12 @@ def vim_performance_state_for_ts(ts)
state = vim_performance_states.detect { |s| s.timestamp == ts }
if state.nil?
# Look for state for current hour in cache
t = Metric::Helper.nearest_hourly_timestamp(Time.now.utc).to_time(:utc)
ts_iso_now = Metric::Helper.nearest_hourly_timestamp(Time.now.utc)
t = ts_iso_now.to_time(:utc)
state = vim_performance_states.detect { |s| s.timestamp == t }
end
# look for state from previous perf_capture_state call
state ||= @states_by_ts[ts_iso_now]
else
state = @states_by_ts[Metric::Helper.nearest_hourly_timestamp(Time.now.utc)]
state ||= vim_performance_states.find_by(:timestamp => ts)
Expand Down
10 changes: 5 additions & 5 deletions spec/models/metric/ci_mixin/state_finders_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
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 }
let(:ts_now) { Time.now.utc.beginning_of_hour }
let(:timestamp) { 2.hours.ago.utc.beginning_of_hour }

describe "#vim_performance_state_association" do
let(:c_vps_now) { create_vps(image, ts_now, :containers => [container1, container2]) }
Expand Down Expand Up @@ -104,8 +104,8 @@
expect(image).to receive(:perf_capture_state).never

expect do
expect(image.vim_performance_state_for_ts(timestamp)).to eq(vps_now) # TODO: should be vps
end.to make_database_queries(:count => 0)
expect(image.vim_performance_state_for_ts(timestamp)).to eq(vps)
end.not_to make_database_queries
end

it "falls back to cached now" do
Expand Down Expand Up @@ -159,7 +159,7 @@
it "creates (and caches) a value when now isn't cached" do
rec_states = VimPerformanceState.where(:timestamp => [ts_now, timestamp])
MiqPreloader.preload(image, :vim_performance_states, rec_states)
expect(image).to receive(:perf_capture_state).twice.and_return(vps_now) # fix
expect(image).to receive(:perf_capture_state).once.and_return(vps_now)

expect do
expect(image.vim_performance_state_for_ts(timestamp)).to eq(vps_now)
Expand Down

0 comments on commit 9c541c7

Please sign in to comment.