-
Notifications
You must be signed in to change notification settings - Fork 28
ensure only one connection acquisition per request #1966
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1966 +/- ##
======================================
Coverage 73.2% 73.2%
======================================
Files 372 372
Lines 13805 13817 +12
Branches 1408 1409 +1
======================================
+ Hits 10114 10125 +11
- Misses 3317 3320 +3
+ Partials 374 372 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
14b09a6
to
3b7890d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solution is very elegant but subtle. If the FastAPI dependency cache gets deactivated or the policy changes it is difficult to notice that all repositories in a request is using the same connection. Add at least a note or a reference to this PR so that in the future we could understand the rationale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, for now. This could also cause issues of requests timing out, because it is taking too long to get a free connection.
What do these changes do?
Some requests were acquiring several connections
This leads to an issue where a burst of requests (concurrent requests) might end in a dead-lock in case where several repositories are needed at the same time by the request and the amount of connection is limited.
For example:
--> request 1 and 2 are locked... since there is no connection available anymore + the 8 remaining requests are waiting forever...
The solution here changes the following:
Pros: no dead-lock, it keeps the nice concept
Cons: in case of bursts, the requests have to wait until a connection is available even though maybe the request would not need to block the connection all the time... to be continued...
BONUS:
Related issue number
How to test
Checklist
make openapi-specs
,git commit ...
and thenmake version-*
)