-
Notifications
You must be signed in to change notification settings - Fork 897
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.
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
MiqQueue consistency with purging #14676
Changes from all commits
d7a1e3a
d9d1942
7e80f6c
28c97a9
ce61ca2
a84c909
ecb49ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,16 +12,6 @@ def purge_window_size | |
::Settings.policy_events.history.purge_window_size | ||
end | ||
|
||
def purge_queue | ||
MiqQueue.put_unless_exists( | ||
:class_name => name, | ||
:method_name => "purge", | ||
:role => "event", | ||
:queue_name => "ems" | ||
) | ||
end | ||
alias_method :purge_timer, :purge_queue | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, is the commit message wrong here?
Are we now using the mixin's method which does a put? All I see is deleted lines here, no new method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's ok, it's coming from the mixin. It wasn't clear when looking at the commit message. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for keeping me honest. Yes, most of these come from the default implementation |
||
def purge_scope(older_than) | ||
where(arel_table[:timestamp].lt(older_than)) | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kbrock Role is not specified any more. Is it by intention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Purging is quick. We perform it as pure sql and do not download tons of records.
So the thought is there is no reason to limit which workers can purge.
But that is my though, wanted your feedback here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, purging is a pure database function and can be done anywhere