Redis retry configuration is potentially confusing

[JetForMe]

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.

[petrpavlik]

Big +1 to accept this update, just spent 30 minutes debugging why is my redis connection timing out.

[fabianfett]

This requires a breaking change. @0xTim could this be fixed in Vapor?

[0xTim]

Yep happy to accept a PR to fix the default in Vapor