Improve connection pool related RedisClientOptions and NetClientOptions parameter documentation

[heubeck]

Describe the feature

The documentation regarding connection pooling is not sufficient and should be more detailed and explaining in my opinion.

Use cases

The configuration related to connection pooling supports the following parameters, please find my comments inline the code block:

NetClientOptions netClientOptions = new NetClientOptions()
            .setConnectTimeout(connectTimeoutMs)

/* documentation says: Connection to be closed if no data is received in this time.
   It it what others call "socketTimeout"?
   How is the behavior related to the 'poolCleanerInterval' and the 'poolRecycleTimeout' of the RedisOptions? */
            .setIdleTimeout(idleTimeoutSeconds)
            .setIdleTimeoutUnit(TimeUnit.SECONDS)

/* Not redis-client specific, but it should be stated out that a connect operation lasts
   (connectTimeout + reconnectAttempts * ReconnectInterval);
   my first thought was that reconnect is about connection aborts after a connection was established */
            .setReconnectAttempts(reconnectAttempts)
            .setReconnectInterval(reconnectIntervalMs);

 Redis redisClient =Redis.createClient(
            get(),
            RedisOptions()

/* What does this setting do at all? Is it the interval, in which the 'poolRecycleTimeout' is check and stale connections are closed?
   Documentation says just "Tune how often in milliseconds should the connection pool cleaner execute."
   But this doesn't explain what it does and what that means. */
                .setPoolCleanerInterval(poolCleanerInterval)

/* This parameter is best explained in docs: "the timeout to keep an open connection on the pool waiting and then close"
   But: is this only relevant if also a 'poolCleanerInterval' is configured (defaults to -1 = inactive).
   And: how does this correlate/clash with the 'idleTimeout' of the NetClientOptions? */
                .setPoolRecycleTimeout(poolRecycleTimeoutMs)

                .setNetClientOptions(netClientOptions)
                .setMaxPoolSize(maxPoolSize)
                .setConnectionString(config.redisUrl);

Contribution

I would love to enhance the JavaDocs as well as the documentation by myself - as soon as I'm certain about these configuration options.

Credits

Thank you for this library, it's fun to use!

[heubeck]

We've found out in the discussion in discord that there are some issues with this settings: https://github.com/vert-x3/vertx-redis-client/issues/289 and https://github.com/vert-x3/vertx-redis-client/issues/290

After this tickets are decided, the documentation should be adjusted.

[pmlopes]

Hi @heubeck all net client options are vertx-core specific, changing them is probably not a good idea as they would introduce a major breaking change to users.

The net client options are related to the TCP level of the connection, they are not related to redis directly. Which leaves us with:

/* What does this setting do at all? Is it the interval, in which the 'poolRecycleTimeout' is check and stale connections are closed?
   Documentation says just "Tune how often in milliseconds should the connection pool cleaner execute."
   But this doesn't explain what it does and what that means. */
                .setPoolCleanerInterval(poolCleanerInterval)

/* This parameter is best explained in docs: "the timeout to keep an open connection on the pool waiting and then close"
   But: is this only relevant if also a 'poolCleanerInterval' is configured (defaults to -1 = inactive).
   And: how does this correlate/clash with the 'idleTimeout' of the NetClientOptions? */
                .setPoolRecycleTimeout(poolRecycleTimeoutMs)

Sadly setPoolRecycleTimeout() was left over during the migration to the new pool and does not have any effect. I've just removed it entirely from the repo.

Regarding setPoolCleanerInterval I've added an extra comment to the javadoc. The use case is, when you have a pool and are not actively ensuring that broken connections are closed (which will remove them from the pool) this timer means that a periodic task is running looking all the connections in a broken state and forcebily closes them.