Add support to pool to do scheduled connection validation

[ifindya]

Provide the kind of functionality that the connection validation is periodically run in the background.

Motivation

Typically, idle connections will become invalid due to timeouts, network side, or the other cause, in which case we potentially would have to retry the acquirement of the whole pool size of connections when performing a database call and it would be even worse if no valid connections exist which would result to re-warmup the pool. That can significantly increase the time of database calls, which could have been avoided if the connection validation was periodically run in the background.

Desired solution

Checking for each connection periodically if it has exceeded its saying maxIdleTime, also check for each connection whether it is valid or not. Also, after the checks, the connection pool should be re-warmuped, so that it contains at least minIdle connections. Some popular libraries like HikariCP (keepaliveTime) and C3P0 (idleConnectionTestPeriod) and Druid(keepAlive) have this feature, which could be referenced.

Considered alternatives

Additional context

[violetagg]

@ifindya There is already such functionality. Can you try this configuration?

reactor-pool/reactor-pool/src/main/java/reactor/pool/PoolBuilder.java

Lines 227 to 249 in 574c24b

	/**
	* Enable background eviction so that {@link #evictionPredicate(BiPredicate) evictionPredicate} is regularly
	* applied to elements that are idle in the pool when there is no pool activity (i.e. {@link Pool#acquire() acquire}
	* and {@link PooledRef#release() release}).
	* <p>
	* Providing an {@code evictionInterval} of {@link Duration#ZERO zero} is similar to {@link #evictInBackgroundDisabled() disabling}
	* background eviction.
	* <p>
	* Background eviction support is optional: although all vanilla reactor-pool implementations DO support it,
	* other implementations MAY ignore it.
	* The background eviction process can be implemented in a best effort fashion, backing off if it detects any pool activity.
	*
	* @return this {@link Pool} builder
	* @see #evictInBackground(Duration, Scheduler)
	*/
	public PoolBuilder<T, CONF> evictInBackground(Duration evictionInterval) {
	if (evictionInterval == Duration.ZERO) {
	this.evictionBackgroundInterval = Duration.ZERO;
	this.evictionBackgroundScheduler = Schedulers.immediate();
	return this;
	}
	return this.evictInBackground(evictionInterval, Schedulers.parallel());
	}

[ifindya]

Thank you so much. I have had a look. background tasks are used to evict idle connections and do not keep the connection active, sometimes needing to contain minIdle idle connections in the connection pool at all times until the pool shutdown, in other words, idle connections are exempt from eviction. Did I get that right?

[violetagg]

@ifindya Yes correct.

For example in Reactor Netty the connection pool has these settings: max idle time, max life time, active/closed connection.
So when the background task is executed these settings are checked and some of the connections might be evicted (removed) from the pool.

If the connection pool supports warm up then after the eviction, if the pool needs to be filled again with fresh connections then this will be performed.

[ifindya]

@violetagg That's to say Reactor Pool doesn't have these settings and might not warm up after the eviction, right? If so, is there a plan about supporting this?

[violetagg]

@violetagg That's to say Reactor Pool doesn't have these settings and might not warm up after the eviction, right? If so, is there a plan about supporting this?

On the contrary - everything as infrastructure/configurations is there, the library that uses Reactor Pool needs to use/configure them.

For example, Reactor Netty configures Reactor Pool like this:
https://github.com/reactor/reactor-netty/blob/61d1315c358e73b5a0c3c3e8ace89917f1a9a2ad/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java#L489-L491

So if background task is enabled the connections will be checked for idle time, life time and whether they are closed.

Reactor Netty does not have configuration for warm up, but if the library that uses Reactor Pool enables the configuration for warm up then the pool will be filled with fresh resources.

[ifindya]

All right, then the implementations should be allowed to extend the background task and determine its behavior. Is there such an ability now?

[violetagg]

All right, then the implementations should be allowed to extend the background task and determine its behavior. Is there such an ability now?

Reactor Pool invokes only the eviction predicate. There is nothing else interesting in the background task.

reactor-pool/reactor-pool/src/main/java/reactor/pool/SimpleDequePool.java

Line 155 in 574c24b

if (evictionPredicate.test(pooledRef.poolable, pooledRef)) {

The library that uses Reactor pool needs to provide eviction predicate.
As I see that you linked an issue in R2DBC - this means R2DBC has to provide this eviction predicate.

[ifindya]

Got it, thanks.

[grantas33]

In r2dbc-pool, when checking whether a connection is valid, a call to the database is made which might take a while. Does reactor-pool ensure that during the eviction check, the pool object (in this case, r2dbc connection) which is being checked for eviction will not be used concurrently by the pool? Also, how to enable the configuration for warmup?

[violetagg]

In r2dbc-pool, when checking whether a connection is valid, a call to the database is made which might take a while. Does reactor-pool ensure that during the eviction check, the pool object (in this case, r2dbc connection) which is being checked for eviction will not be used concurrently by the pool?

yes

Also, how to enable the configuration for warmup?

reactor-pool/reactor-pool/src/main/java/reactor/pool/PoolBuilder.java

Lines 346 to 362 in 574c24b

	/**
	* Replace the {@link AllocationStrategy} with one that lets the {@link Pool} allocate between {@code min} and {@code max} resources.
	* When acquiring and there is no available resource, the pool should strive to warm up enough resources to reach
	* {@code min} live resources before serving the acquire with (one of) the newly created resource(s).
	* At the same time it MUST NOT allocate any resource if that would bring the number of live resources
	* over the {@code max}, rejecting further allocations until some resources have been {@link PooledRef#release() released}.
	*
	* @param min the minimum number of live resources to keep in the pool (can be best effort)
	* @param max the maximum number of live resources to keep in the pool. use {@link Integer#MAX_VALUE} when you only need a
	* minimum and no upper bound
	* @return this {@link Pool} builder
	* @see #sizeUnbounded()
	* @see #allocationStrategy(AllocationStrategy)
	*/
	public PoolBuilder<T, CONF> sizeBetween(int min, int max) {
	return allocationStrategy(new AllocationStrategies.SizeBasedAllocationStrategy(min, max));
	}

[ifindya]

@ifindya Yes correct.

For example in Reactor Netty the connection pool has these settings: max idle time, max life time, active/closed connection. So when the background task is executed these settings are checked and some of the connections might be evicted (removed) from the pool.

If the connection pool supports warm up then after the eviction, if the pool needs to be filled again with fresh connections then this will be performed.

I am sorry we're not on the same page before. The library depends on Reactor pool uses eviction predicate to check whether connection is valid and that would require obtaining the connection from the pool, testing it, and removing it from the pool if invalid, thus connection validation might be a remote call(saying to send a message to server side) in non-block way and BiPredicate is incompatible. It seems that there is only one way that will be performed after the eviction is to provide DestoryHandler, which allows users to have effects on the pool from the outside. Therefore, it's not exactly a walk in the park to combine background eviction and warm up. Thanks for your reply in advance.