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.

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

Add a config for enforcing a minimum job reserve interval (default: 0 for now) #48

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

smudge
Copy link
Member

@smudge smudge commented Dec 18, 2024

This is related to #41 and #42 insofar as a kind of DB-bound "spinloop" is still possible if a worker picks up jobs that take so little time that the worker immediately turns around and asks for more. As of now, there has been no way to tune the amount of time a worker should wait in between successful iterations of its run loop.

This introduces a configuration (min_reserve_interval) specifying a minimum number of seconds (default: 0 as this is not a major release) that a worker should wait in between successful job reserve queries. An existing config (sleep_delay) is still used to define the number of seconds (default: 5) that a worker should wait in between unsuccessful job reserve attempts (i.e. the queue is empty).

The job execution time is subtracted from min_reserve_interval when the worker sleeps, and if jobs take more than min_reserve_interval to complete than the worker will not sleep before the next reserve query.

/no-platform

effron
effron previously approved these changes Dec 18, 2024
Copy link
Contributor

@effron effron left a comment

Choose a reason for hiding this comment

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

domainLGTM!

@smudge smudge requested review from rzane and pat-whitrock December 18, 2024 16:03
@jmileham
Copy link
Member

Should a default behavior change like this be reserved for a major version? Like introduce the param at 0 seconds in 0.6 and then pop it to 1s at 1.0?

@smudge smudge requested a review from jmileham December 18, 2024 17:00
@smudge smudge changed the title Enforce a minimum job reserve interval of 1 second (configurable) Add a config for enforcing a minimum job reserve interval (default: 0 for now) Dec 18, 2024
Copy link
Member

@jmileham jmileham left a comment

Choose a reason for hiding this comment

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

domain lgtm too! Will leave platform to somebody closer/more current, if we want that. But I also think Nathan's currency satisfies that requirement for me for no-platform.

@smudge smudge merged commit c8ac357 into Betterment:main Dec 18, 2024
20 checks passed
@smudge smudge deleted the sleep-on-success branch December 18, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants