-
Notifications
You must be signed in to change notification settings - Fork 2
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
Investigate performance issues introduced by 9db3e05
#235
Comments
9db3e05
](https://github.com/vipyrsec/dragonfly-mainframe/commit/9db3e05e6a728abd48d460eae13cda3d2545c5cf)9db3e05
we get a deadlock caused by fastapi scheduling the dependency cleanup in the threadpool shared by endpoint functions. the reason we deadlock is that we have a limited number of database connections, This was discussed in this issue: tiangolo/fastapi#6628, leading to a PR: tiangolo/fastapi#5122, released in version As a stop-gap solution, we can simply manually close sessions ourselves, either using context managers or just |
Further investigation in the Discord server yields a peculiar issue relating to deadlocks. There are a handful of discussions, issues, and PRs that are related to this:
These being merged and closed implies that this issue was fixed - however this does not seem to be the case (at least for us). The issue, quite intuitive in retrospect, boils down to the following: we get some number of requests coming in (let's say 50), which is greater than the amount of internal thread pool workers (let's say 40 worker threads). All 40 worker threads start working on their requests, Only some of these will run queries (let's say we have 5 database connections). The rest of the 35 worker threads will block for a connection to become available. Once the first 5 requests are done with their queries, they schedule 5 tasks to close their sessions. However, since all workers in the threadpool are currently busy (either waiting for their cleanup tasks to run, or waiting for a database connection), the cleanup tasks cannot run - therefore, we deadlock. The most straightforward solution is to not rely on FastAPI's dependency injection system for database connections, and simply make sessions ourselves when we need it and close it at the end of the path operation. This should ensure that we don't create a whole new task just for closing sessions that could possibly deadlock us. |
This script runs sucessfully prior to 9db3e05
However, running it with 9db3e05 checked out results in SQLAlchemy throwing a
QueuePool limit of size 5 overflow 17 reached, connection timed out, timeout 30.00
error (background)The text was updated successfully, but these errors were encountered: