Skip to content

Limit max. sleep duration per loop iteration #42

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

Closed
wants to merge 2 commits into from
Closed

Limit max. sleep duration per loop iteration #42

wants to merge 2 commits into from

Conversation

mvorisek
Copy link
Member

max. sleep duration should be based on timeout, as for shorter timeouts, we should loop more often (like for web/API) vs. some long timeouts (for CLI/crons)

@willemstuursma
Copy link
Contributor

Sorry, it is not really clear to me what you are trying to achieve. Can you elaborate in your initial comment?

@mvorisek
Copy link
Member Author

mvorisek commented Dec 1, 2020

@willemstuursma if you set timeout to some "interactive time" like 30 seconds, we should not wait between next retry too much. On the other side, this PR increases the retry time up to 25 seconds for long timeouts/processes.

In other words, this PR improves interactivity for interactive applications and save resources when lock takes a lot of time to aquire (and is allowed to - never more than timeout / 120).

@mvorisek
Copy link
Member Author

mvorisek commented Jan 6, 2021

@willemstuursma please review this PR with my explanation and either merge or concretize your feedback I can address, thanks

@willemstuursma
Copy link
Contributor

Hi @mvorisek.

Thank you for explaining what your code does. However, I am missing any clear advantages backed up by numbers (or even anecdotes).

You change is highly arbitrary, I feel that the goals it aims to achieve are already achieved by the existing exponential back off.

I cannot really phantom a situation where you would have so many competing threads that a polling interval > 500ms is required. IMHO, the underlying storage engines (Redis, memcached) can easily handle existing situations.

@mvorisek mvorisek deleted the better_sleep_time branch January 11, 2021 18:09
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.

2 participants