Skip to content

Commit

Permalink
vim_performance_state_for_ts(Time)
Browse files Browse the repository at this point in the history
defaulting to passing time into the method.
Places calling it, ensure that

Fixing:

sometimes we were passing non iso8061 into the method
so it was not finding the cached VimPerformanceState records

|       ms |       bytes |   objects |query |  qry ms |     rows |` comments`
|      ---:|         ---:|       ---:|  ---:|     ---:|      ---:|  ---
|  1,699.1 | 19,618,021* |   283,526 |    9 | 1,519.6 |       79 |`before`
|  1,664.1 | 15,672,327* |   225,826 |    5 | 1,550.8 |       75 |`after-fetch-time`
  • Loading branch information
kbrock committed Jun 27, 2023
1 parent 8840cd9 commit 555ed3b
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 12 deletions.
2 changes: 1 addition & 1 deletion app/models/chargeback/consumption_with_rollups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def chargeback_container_labels

def container_tag_list_with_prefix
if resource.kind_of?(ContainerImage)
state = resource.vim_performance_state_for_ts(timestamp.to_s)
state = resource.vim_performance_state_for_ts(timestamp)
image_tag_name = "#{state.image_tag_names}|" if state

image_tag_name.split("|")
Expand Down
4 changes: 4 additions & 0 deletions app/models/metric/ci_mixin/capture.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ def calculate_gap(interval_name, start_time)
end
end

# @param interval_name ["realtime", "hourly", "historical"]
# @param start_time [String|nil] start time for historical capture (nil for all other captures)
# @param end_time [String|nil] end time for historical capture (nil for all other captures)
# @param metrics_capture [MetricCapture]
def just_perf_capture(interval_name, start_time, end_time, metrics_capture)
log_header = "[#{interval_name}]"
log_time = ''
Expand Down
25 changes: 18 additions & 7 deletions app/models/metric/ci_mixin/state_finders.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
module Metric::CiMixin::StateFinders
# load from cache or create a VimPerformanceState for a given timestamp
#
# for a cache:
# use preload to populate vim_performance_states associations
# this loads all records
# the cache is indexed by a Time object
# used by Metric.rollup / rollup_hourly
# preload_vim_performance_state_for_ts_iso8601 to populate @states_by_ts
# contains a subset, typically now 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
def vim_performance_state_for_ts(ts)
ts = Time.parse(ts).utc if ts.kind_of?(String)
ts_iso = ts.utc.iso8601
return nil unless self.respond_to?(:vim_performance_states)

@states_by_ts ||= {}
state = @states_by_ts[ts]
state = @states_by_ts[ts_iso]
if state.nil?
# TODO: vim_performance_states.loaded? works only when doing resource.vim_performance_states.all, not when loading
# a subset based on available timestamps
# using preloaded vim_performance_states association
if vim_performance_states.loaded?
# Look for requested time in cache
t = ts.to_time(:utc)
state = vim_performance_states.detect { |s| s.timestamp == t }
state = vim_performance_states.detect { |s| s.timestamp == ts }
if state.nil?
# Look for state for current hour in cache if still nil because the
# capture will return a state for the current hour only.
# Look for state for current hour in cache
t = Metric::Helper.nearest_hourly_timestamp(Time.now.utc).to_time(:utc)
state = vim_performance_states.detect { |s| s.timestamp == t }
end
Expand Down
13 changes: 9 additions & 4 deletions app/models/metric/rollup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,13 @@ def self.rollup_realtime_perfs(obj, rt_perfs, new_perf = {})
new_perf
end

# @param obj parent object
# @param timestamp [Time]
# @param interval_name ["realtime", "hourly", "historical"]
# @param assoc [Symbol] name of the association to rollup
def self.rollup_child_metrics(obj, timestamp, interval_name, assoc)
ts = timestamp.kind_of?(Time) ? timestamp.utc.iso8601 : timestamp
timestamp = Time.parse(timestamp).utc if timestamp.kind_of?(String)
ts_iso = timestamp.utc.iso8601
recs = obj.vim_performance_state_association(timestamp, assoc).to_a

result = {}
Expand All @@ -280,7 +285,7 @@ def self.rollup_child_metrics(obj, timestamp, interval_name, assoc)
result[c] = 0
end

perf_recs = Metric::Finders.hash_by_capture_interval_name_and_timestamp(recs, ts, ts, interval_name)
perf_recs = Metric::Finders.hash_by_capture_interval_name_and_timestamp(recs, ts_iso, ts_iso, interval_name)

# Preload states for perf timestamp and the current hour.
# in a single query bring back all relevant performance states
Expand All @@ -290,15 +295,15 @@ def self.rollup_child_metrics(obj, timestamp, interval_name, assoc)
# For infra, if that record is not found, use the current performance_state (can't capture in the past)
if recs.present?
rec_states = VimPerformanceState.where(
:timestamp => [ts, Metric::Helper.nearest_hourly_timestamp(Time.now.utc)],
:timestamp => [timestamp, Metric::Helper.nearest_hourly_timestamp(Time.now.utc)],
:resource_type => recs.first.class.base_class.name,
:resource_id => recs.map(&:id)
)
MiqPreloader.preload(recs, :vim_performance_states, rec_states)
end

recs.each do |rec|
perf = perf_recs.fetch_path(rec.class.base_class.name, rec.id, interval_name, ts)
perf = perf_recs.fetch_path(rec.class.base_class.name, rec.id, interval_name, ts_iso)
next unless perf
state = rec.vim_performance_state_for_ts(timestamp)
agg_cols.each do |c|
Expand Down
2 changes: 2 additions & 0 deletions app/models/vim_performance_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class VimPerformanceState < ApplicationRecord
# => host_sockets (derive from assoc_ids)

# TODO: do a single query for the finds
# @param objs [Array[Object]|Object] MetricsCapture#target - all should be the same object class
# @returns [Array[VimPerformanceState]|VimPerformanceState] return an array if an array was passed in
# NOTE: a few calls (with a single object) do use the return and expect a single result
def self.capture(objs)
ts = Time.now.utc
Expand Down
10 changes: 10 additions & 0 deletions spec/models/vim_performance_state_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
RSpec.describe VimPerformanceState do
describe ".capture" do
it "uses now for Infra" do
host = FactoryBot.create(:host)
state = VimPerformanceState.capture(host)

expect(state.timestamp).to be_within(1.minute).of(Time.now.utc.beginning_of_hour)
expect(state.capture_interval).to eq(3600) # 1.hour
end
end

describe ".capture_host_sockets" do
it "returns the host sockets when given a host" do
hardware = FactoryBot.build(:hardware, :cpu_sockets => 2)
Expand Down

0 comments on commit 555ed3b

Please sign in to comment.