Skip to content

More predictable timeout-notify with slow sync calls #2625

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ollanta
Copy link

@ollanta ollanta commented May 5, 2025

Summary

The timeout-notify mechanism is less predictable than necessary when slow synchronous calls are run on the event loop thread. This can lead to uvicorn workers being killed by (for example) gunicorn while they're still responsive.

The uvicorn servers main_loop triggers on_tick and sleeps asynchronously for 0.1 seconds. Every 10 ticks on_tick checks if it's time to run cfg.callback_notify. The comments in the code imply that the check is supposed to run once every second, but if there are synchronous calls blocking the asynchronous sleeps from triggering in a timely manner, the actual cadence can be a lot slower.

Of course, you're not supposed to block the event loop like that, but checking the need to notify on each tick (every 0.1+ seconds) improves the situation at a negligible cost.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@ollanta ollanta changed the title More predictable timeout More predictable timeout with slow sync calls May 5, 2025
@ollanta ollanta changed the title More predictable timeout with slow sync calls More predictable timeout-notify with slow sync calls May 5, 2025
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.

1 participant