Timeout connection requests when in a deadlock. #67
Conversation
Codecov Report
@@ Coverage Diff @@
## master #67 +/- ##
==========================================
+ Coverage 94.66% 94.76% +0.10%
==========================================
Files 23 23
Lines 1180 1204 +24
==========================================
+ Hits 1117 1141 +24
Misses 63 63
Continue to review full report at Codecov.
|
|
|
||
| let task = eventLoop.scheduleTask(in: self.creationTimeout) { [weak self, logger, promise] in | ||
| logger.error("Connection creation timed out. This might indicate a connection deadlock in your application.") | ||
| if let idx = self?.waiters.firstIndex(where: { $0.2 == uuid }) { |
There was a problem hiding this comment.
Both the search and the remove would have linear runtime each. That is not ideal, but hopefully this occurs rarely enough to not be a bottleneck.
Unless in those 10 seconds before timing out so many waiters accumulate that it actually does become a bottleneck... When stuff fails, it often all fails at once, after all.
| promise.fail(ConnectionPoolTimeoutError.connectionRequestTimeout) | ||
| } | ||
|
|
||
| return promise.futureResult.always { _ in task.cancel() } |
There was a problem hiding this comment.
Is cancelling a task that has already completed fine?
There was a problem hiding this comment.
I did a quick test where the task would always finish before cancelling and didn't see anything going wrong, so guessing it's fine. Not 100% sure though.
cc @Lukasa
|
These changes are now available in 1.2.0 |
Fixes a bug when requesting
n+1connections from a connection pool with limitnwhere each new connection waited on the previous one.Previously, this would create a deadlock. Now, a timeout can be added to
EventLoopConnectionPool, that will return a failed future if the timeout is reached. Timeout defaults to 10 seconds but can be configured.Fixes #63