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

Throttler: sleep without busy wait, log delayed calls #1526

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

chatziko
Copy link
Contributor

This PR adds two improvements to throttling:

  1. Accurate sleep until the next available time. asyncio_throttle by default wakes every 10msecs which is crazy. ThrottlerManager sets the wait to 100mecs, which is still unnecessary, but some modules don't use ThrottlerManager anyway.

  2. Return the delay caused by the throttler (0 means no throttling) and log it in throttle_with_retries.

Since asyncio_throttle is very simple and looks unmaintained (PR ignored for years) I copied the code in server/helpers/throttle_retry.py, no point having an unmaintained dependency for a few lines of code.

marcelveldt
marcelveldt previously approved these changes Jul 29, 2024
Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

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

Looks good, nice work!

@marcelveldt
Copy link
Member

CI found an issue in the code, can you check that please ?

Tip: to setup the dev environment use the scripts\setup.sh

@chatziko chatziko force-pushed the improve-throttling branch from 65d8066 to 782e017 Compare July 29, 2024 11:56
@chatziko
Copy link
Contributor Author

That's strange, the pre-commit checks are passing in my vscode dev container. Even stranger is that the failing line was not changed by me but by the ruff formatter! But now I ran the formatter again and it didn't make the same change. Weird...

Anyway I recommitted without the formatting changes and it seems to pass now.

@marcelveldt
Copy link
Member

Maybe a version mismatch of the ruff formatter in your dev container and CI ?

Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@marcelveldt marcelveldt merged commit 289fe77 into music-assistant:main Jul 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants