Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix queueing of historical metrics collection #14695

Merged
merged 5 commits into from
Apr 10, 2017
Merged

Fix queueing of historical metrics collection #14695

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
155ddd6
Fix queueing of historical metrics collection
gtanzillo Apr 7, 2017
8b5bf74
Splitting realtime collection into historical
blomquisg Apr 7, 2017
baed845
More splitting realtime and tests
blomquisg Apr 8, 2017
9bf613e
Fix RuboCop - Use `update_attributes` instead of `update_attribute`
gtanzillo Apr 8, 2017
004569f
Handle case where last_perf_capture_on is nil and just queue up a sin…
gtanzillo Apr 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions app/models/metric/ci_mixin/capture.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,28 @@ def perf_capture_queue(interval_name, options = {})

log_target = "#{self.class.name} name: [#{name}], id: [#{id}]"

# Determine the items to queue up
# cb is the task used to group cluster realtime metrics
cb = nil
items = []
if interval_name == 'historical'
start_time = Metric::Capture.historical_start_time if start_time.nil?
end_time = Time.now.utc if end_time.nil?
end_time ||= 1.day.from_now.utc.beginning_of_day # Ensure no more than one historical collection is queue up in the same day
items = split_capture_intervals(interval_name, start_time, end_time)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

else
start_time = last_perf_capture_on unless start_time
end_time = Time.now.utc unless end_time
# if last_perf_capture_on is earlier than 4.hour.ago.beginning_of_day,
# then create *one* realtime capture for start_time = 4.hours.ago.beginning_of_day (no end_time)
# and create historical captures for each day from last_perf_capture_on until 4.hours.ago.beginning_of_day
realtime_cut_off = 4.hours.ago.utc.beginning_of_day
if last_perf_capture_on && last_perf_capture_on < realtime_cut_off
items = split_capture_intervals("historical", last_perf_capture_on, realtime_cut_off)
end
# push realtime item to the front of the array
items.unshift([interval_name, realtime_cut_off])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we only want to do this when the time is outside the realtime_cut_off

that is, I think something like

realtime_cut_off = 4.hours.ago.utc.beginning_of_day
if last_perf_capture_on && last_perf_capture_on < realtime_cut_off
  items = [[interval_name, realtime_cut_off]] +
    split_capture_intervals("historical", last_perf_capture_on, realtime_cut_off)
else
  items = [interval_name]
end

Copy link
Member

@blomquisg blomquisg Apr 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of VMware this will be fine, I think. Since the max we can get from the provider is 4.hours of data. However, for Hawkular, I think this breaks, since the max we can get from the provider is 7.days.

So, without a start date, I think it will try to get the maximum amount of data available. But, I'm not positive.

Edit

And, that leads us back to the "we should fix this in perf_capture instead of perf_capture_queue" discussion...


cb = {:class_name => self.class.name, :instance_id => id, :method_name => :perf_capture_callback, :args => [[task_id]]} if task_id
end

# Determine what items we should be queuing up
items = [interval_name]
items = split_capture_intervals(interval_name, start_time, end_time) if start_time

# Queue up the actual items
queue_item = {
:class_name => self.class.name,
Expand All @@ -78,15 +86,17 @@ def perf_capture_queue(interval_name, options = {})
if msg.nil?
qi[:priority] = priority
qi.delete(:state)
qi[:miq_callback] = cb if cb
if cb and item_interval == "realtime"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer && over and ;)

qi[:miq_callback] = cb
end
qi
elsif msg.state == "ready" && (task_id || MiqQueue.higher_priority?(priority, msg.priority))
qi[:priority] = priority
# rerun the job (either with new task or higher priority)
qi.delete(:state)
if task_id
existing_tasks = (((msg.miq_callback || {})[:args] || []).first) || []
qi[:miq_callback] = cb.merge(:args => [existing_tasks + [task_id]])
qi[:miq_callback] = cb.merge(:args => [existing_tasks + [task_id]]) if item_interval == "realtime"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can get rid of the check on upgrading priority, then it will be easier to go over to put_unless_exists.

Real-time will potentially still need this

end
qi
else
Expand Down Expand Up @@ -177,6 +187,8 @@ def perf_capture(interval_name, start_time = nil, end_time = nil)

if start_range.nil?
_log.info "#{log_header} Skipping processing for #{log_target} as no metrics were captured."
# Set the last capture on to end_time to prevent forever queueing up the same collection range
update_attribute(:last_perf_capture_on, end_time) if interval_name == 'realtime'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be update_attributes instead of update_attribute

else
if expected_start_range && start_range > expected_start_range
_log.warn "#{log_header} For #{log_target}, expected to get data as of [#{expected_start_range}], but got data as of [#{start_range}]."
Expand Down