-
Notifications
You must be signed in to change notification settings - Fork 190
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 poll-interval-variance option #431
base: master
Are you sure you want to change the base?
Conversation
@@ -275,7 +275,7 @@ def assert_poll(priorities:, locked:) | |||
assert_equal false, poller.should_poll? | |||
end | |||
|
|||
it "should be false if the number of jobs returned from the last poll was less than the lowest priority request, but the poll_interval has elapsed" do | |||
it "should be true if the number of jobs returned from the last poll was less than the lowest priority request, but the poll_interval has elapsed" do |
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.
That was a typo, the test is actually checking assert_equal true
, which is the expected behavior.
job_ids = 5.times.map { Que::Job.enqueue.que_attrs[:id] } | ||
|
||
result = poller.poll(priorities: { 500 => 7 }, held_locks: Set.new) | ||
assert_equal job_ids, result.map(&:id) | ||
|
||
poller.instance_variable_set(:@last_polled_at, Time.now - 30) | ||
assert_equal true, poller.should_poll? | ||
Timecop.freeze(Time.now + 30) do |
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.
This change makes the spec less coupled with implementation details.
Instead of setting private instance variables, it 'time travels' 30 seconds into the future, after the poll_interval
has already elapsed.
c23171b
to
5e50c41
Compare
5e50c41
to
6ccd8bc
Compare
Enable this for cleaner diff:
Context
We run multiple ECS tasks of que that are all started almost exactly at the same time during every deployment.
That means that they also start polling at the same time, and continue doing so in such a synchronized fashion.
More specifically, in our case:
poll-interval
is set to 20sMeaning that on average we poll every 5s, but in reality, we poll 4 times every 20s 🙂
We would like to introduce a random variance to the polling interval, so that the polling is more evenly distributed.
Changes
Add optional
poll-interval-variance
CLI parameter defaulting to 0.0.It is used to make each individual poll time slightly randomised within the range of
poll-interval +/- poll-interval-variance
.This way tasks started at the same time won't continue always polling at the same time.
Backward-compatibility
When the new
poll-interval-variance
parameter is not provided, there's no change compared to the current behaviour.