Redis retry configuration is potentially confusing
The Redis retry configuration is potentially confusing, and downright problematic for Vapor.
RedisConnectionPool.Configuration.init() is implemented like this. Note that it specifies a default timeout in the signature of 60 seconds, but if nil is passed in, it uses an internal timeout of 10 ms.
public init(
initialServerConnectionAddresses: [SocketAddress],
maximumConnectionCount: RedisConnectionPoolSize,
connectionFactoryConfiguration: ConnectionFactoryConfiguration,
minimumConnectionCount: Int = 1,
connectionBackoffFactor: Float32 = 2,
initialConnectionBackoffDelay: TimeAmount = .milliseconds(100),
connectionRetryTimeout: TimeAmount? = .seconds(60),
onUnexpectedConnectionClose: ((RedisConnection) -> Void)? = nil,
poolDefaultLogger: Logger? = nil
) {
self.initialConnectionAddresses = initialServerConnectionAddresses
self.maximumConnectionCount = maximumConnectionCount
self.factoryConfiguration = connectionFactoryConfiguration
self.minimumConnectionCount = minimumConnectionCount
self.connectionRetryConfiguration = (
(initialConnectionBackoffDelay, connectionBackoffFactor),
connectionRetryTimeout ?? .milliseconds(10) // always default to a baseline 10ms
)
self.onUnexpectedConnectionClose = onUnexpectedConnectionClose
self.poolDefaultLogger = poolDefaultLogger ?? .redisBaseConnectionPoolLogger
}
The Vapor wrapper around this passes nil down through its call stack.
It's not clear what the intent is in RediStack. Is a default timeout 60 seconds or 10 milliseconds? Certainly the public documentation implies it's 60 seconds, and only by a careful reading of the actual code can you determine that it ends up as 10 ms if you pass nil.
Big +1 to accept this update, just spent 30 minutes debugging why is my redis connection timing out.
This requires a breaking change. @0xTim could this be fixed in Vapor?
Yep happy to accept a PR to fix the default in Vapor