-
Notifications
You must be signed in to change notification settings - Fork 0
draft: Skip poll based on buffer fullness #2
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
Conversation
2076b95 to
7a7ca9e
Compare
7a7ca9e to
1d66a70
Compare
| assert_equal 1, locker_polled_events.size | ||
|
|
||
| # Should have locked first 11 only because there are 8 buffer slots and 3 open workers | ||
| assert_equal ids[0...11], locked_ids |
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.
is 0..11 12 or 11? I think it is 12 right?
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.
.. vs ...
> (0..100).to_a[0..11]
=> [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]
> (0..100).to_a[0...11]
=> [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]This spec is hugely copy-pasted inspired by the one above
Lines 422 to 423 in a58d350
| # Should have locked first 11 only. | |
| assert_equal ids[0...11], locked_ids |
| locker_polled_events = internal_messages(event: 'poller_polled') | ||
| assert_equal 1, locker_polled_events.size | ||
|
|
||
| 3.times { $q2.push 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.
No idea what it does, but 🙈 since it hangs without it and 5 other tests also have it 🤷♂️
I'm going to follow the existing pattern and resist shaving this yak 🙂
Probably something to do with this mutex
Line 215 in 1d66a70
| @cv.wait(mutex) |
push signals Lines 248 to 252 in 1d66a70
| def _push(item) | |
| Que.assert(waiting_count > 0) | |
| @items << item | |
| @cv.signal | |
| end |
lee-treehouse
left a comment
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 think this is a great idea
|
Opened a draft in que-rb#441 |
Hide whitespace changes for much cleaner diff
Draft to get feedback before looking to raise a PR at https://github.com/que-rb/que
Diff on top of our existing changes: wrap_job_in_rails_executor_and_poll-interval-variance...seek-pass-oss:que:wrap_job_in_rails_executor_and_poll-interval-variance_and_skip_poll_based_on_buffer_fullness
Context
During high-throughput spike periods, Que's job polling mechanism can lead to very high database load with continuous polling, even when the job buffer is already nearly full and workers are actively processing jobs.
What is possible to be happening currently:
Changes
Add optional
skip-poll-when-buffer-above-thresholdCLI parameter defaulting to 1.0.When the buffer already has at least
skip-poll-when-buffer-above-thresholdfraction of its capacity filled, the polling is skipped for that iteration.Backward-compatibility
Currently, the polling is skipped only when the buffer is completely full.
When the new
skip-poll-when-buffer-above-thresholdparameter is not provided, it defaults to 1.0 so there's no change compared to the current behaviour.