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

Switch I/O bound path operation functions to non-async #1166

Merged
merged 6 commits into from
Nov 8, 2024
Merged

Conversation

udgover
Copy link
Collaborator

@udgover udgover commented Nov 8, 2024

This PR resolves web api performance issues by removing async usage on I/O bound (ArangoDB calls) path operation functions. Since backend does not support async calls, web api was not running concurrently. Switching to non-async definitions lets FastAPI run calls in awaited thread pool as explained in documentation:

If you are using a third party library that communicates with something (a database, an API, the file system, etc.) and doesn't have support for using await, (this is currently the case for most database libraries), then declare your path operation functions as normally, with just def.

Regarding technical details, this is how FastAPI handles concurrency with non-async path operation functions:

When you declare a path operation function with normal def instead of async def, it is run in an external threadpool that is then awaited, instead of being called directly (as it would block the server).

Some path operation functions are still async because:

  • they call others async functions and make non time-consuming calls to ArangoDB.
  • they do not perform any ArangoDB calls.

@udgover udgover requested a review from tomchop November 8, 2024 08:17
@udgover udgover added enhancement code health Changes about syntax, code health, etc. python Pull requests that update Python code api labels Nov 8, 2024
@udgover udgover linked an issue Nov 8, 2024 that may be closed by this pull request
Copy link
Collaborator

@tomchop tomchop left a comment

Choose a reason for hiding this comment

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

LGTM

@udgover udgover merged commit 6f01784 into main Nov 8, 2024
3 checks passed
@udgover udgover deleted the api_real_async branch November 8, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api code health Changes about syntax, code health, etc. enhancement python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define returned value for all API endpoints
2 participants