Skip to content

Commit

Permalink
Add default buffer connectionRetryTimeout to avoid literal immediate …
Browse files Browse the repository at this point in the history
…timeouts

Motivation:

When trying to allow users to configure the connection retry timeout offset,
not having a value provided (deadline of `now`) caused all attempts to use the pool to fail.

Modifications:

- Change: RedisConnectionPool to always have a timeout offset defined

Result:

If users don't specify any value, then the default of 60 seconds will be used.

If users specify "nil" (or `.none`) as the timeout, then a minimum of 10 milliseconds will be used to avoid immediate timeouts

Otherwise, use the user's specified `TimeAmount` as the offset of the timeout
  • Loading branch information
Mordil committed Oct 7, 2020
1 parent 3e28e75 commit c8cb256
Showing 1 changed file with 3 additions and 7 deletions.
10 changes: 3 additions & 7 deletions Sources/RediStack/RedisConnectionPool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class RedisConnectionPool {
/// This needs to be a var because we reuse the same connection
private var pubsubConnection: RedisConnection?

private let connectionRetryTimeout: TimeAmount?
private let connectionRetryTimeout: TimeAmount
private let connectionPassword: String?
private let connectionSystemContext: Logger
private let poolSystemContext: Context
Expand Down Expand Up @@ -88,7 +88,7 @@ public class RedisConnectionPool {
self.loop = loop
self.serverConnectionAddresses = ConnectionAddresses(initialAddresses: serverConnectionAddresses)
self.connectionPassword = connectionPassword
self.connectionRetryTimeout = connectionRetryTimeout
self.connectionRetryTimeout = connectionRetryTimeout ?? .milliseconds(10)

// mix of terminology here with the loggers
// as we're being "forward thinking" in terms of the 'baggage context' future type
Expand Down Expand Up @@ -392,11 +392,7 @@ extension RedisConnectionPool: RedisClientWithUserContext {
let logger = self.prepareLoggerForUse(context)

guard let connection = preferredConnection else {
return pool
.leaseConnection(
deadline: self.connectionRetryTimeout.map({ .now() + $0 }) ?? .now(),
logger: logger
)
return pool.leaseConnection(deadline: .now() + self.connectionRetryTimeout, logger: logger)
.flatMap { operation($0, pool.returnConnection(_:logger:), logger) }
}

Expand Down

0 comments on commit c8cb256

Please sign in to comment.