Avoid starting connection timeout when a connection is already available#9600
Avoid starting connection timeout when a connection is already available#9600
Conversation
CodSpeed Performance ReportMerging #9600 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #9600 +/- ##
=======================================
Coverage 98.62% 98.63%
=======================================
Files 113 113
Lines 35299 35306 +7
Branches 4189 4191 +2
=======================================
+ Hits 34814 34823 +9
+ Misses 325 323 -2
Partials 160 160
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply e6187f6 on top of patchback/backports/3.11/e6187f6808d647243d8cee115ec24c3eb1421183/pr-9600 Backporting merged PR #9600 into master
🤖 @patchback |
…hen a connection is already available (#9607)
|
Tagging for backport to 3.10 as well since we need it to fix #9670 since a deadlock takes precedence of the potential risks from this PR |
Backport to 3.10: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply e6187f6 on top of patchback/backports/3.10/e6187f6808d647243d8cee115ec24c3eb1421183/pr-9600 Backporting merged PR #9600 into master
🤖 @patchback |
…hen a connection is already available (#9673)
What do these changes do?
Since connection re-use is common, we can avoid starting the
ceil_timeoutwhen we have a connection immediately available. Previously when a connection is already available for reuse,ceil_timeouthad to create aTimerHandle, schedule it on the event loop, and then immediately cancel it, leaving the loop to have to remove it from the heap. See #8608 (comment) and #8613 for more history on the overhead of this set of operations.If the server supports keep-alive, the hit rate on avoiding the timeout is over 80% in testing. 31b9637
closes #9598
Are there changes in behavior for the user?
If
BaseConnector.connectwas replaced with a custom connect function, the timeout must now be handled in this function. The only abstract method the docs specify to be overwritten is_create_connection. While it seems extremely unlikely that someone would write out a customconnectfunction since it would likely break all the protected calls back into the connector fromConnection, I think this change is likely safe to backport.To be on the safe side I only tagged it for 3.11 (tested in #9601) in case someone is doing that.This was originally planned not to backport to 3.10, but its needed to fix #9670Is it a substantial burden for the maintainers to support this?
no
Benchmarks
See script in #9598
aiohttp 3.10.0, yarl 1.9.5

aiohttp 3.11.0b0 yarl 1.17.0

aiohttp 3.11.0b0+ this change yarl 1.17.0

aiohttp 3.11.0b0+ this change yarl 1.17.1
