-
Notifications
You must be signed in to change notification settings - Fork 898
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
Introduce purge timer for drift states #13086
Conversation
@@ -182,6 +182,12 @@ def schedules_for_scheduler_role | |||
enqueue :storage_authentication_check_schedule | |||
end | |||
|
|||
# Schedule - Prune old reports Timer |
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 Code comments: https://twitter.com/nzkoz/status/538892801941848064
@@ -105,6 +105,10 @@ def metric_purging_purge_rollup_timer | |||
queue_work(:class_name => "Metric::Purging", :method_name => "purge_rollup_timer", :zone => nil) | |||
end | |||
|
|||
def drift_state_purge_timer | |||
queue_work(:class_name => "DriftState", :method_name => "purge_timer", :zone => nil) |
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.
What is the default "age" of drift states that it will purge?
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.
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.
I made that up. if you have something else, let me know
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.
Checking once a day seems reasonable.
c13bf58
to
f84a425
Compare
Checked commit kbrock@f84a425 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
@@ -182,6 +182,11 @@ def schedules_for_scheduler_role | |||
enqueue :storage_authentication_check_schedule | |||
end | |||
|
|||
every = worker_setting_or_default(:drift_state_purge_interval, 1.day) |
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.
@yrudman FYI, here is another place that I think you can kill defaults and rely on the settings.yml. worker_setting_or_default
was written to encode defaults in case the key doesn't exist, but with the new config it always will.
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.
yes. this has outlived its usefulness
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.
ok, will look at it
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.
PR: #13157
LGTM 👍 |
Introduce purge timer for drift states (cherry picked from commit f638c97) https://bugzilla.redhat.com/show_bug.cgi?id=1410846
Euwe backport details:
|
This schedules purging drift states
This is contintinuing the trend of #13044 that is addressing
https://bugzilla.redhat.com/show_bug.cgi?id=1348625
Fixes #11868
numbers
in my test, purging 116,576 drift states (with default 10k batch sizes) takes 9 seconds