Provide more configuration properties for Jedis's connection pool

[yhmingyue]

spring-boot version: 2.7.7

When the timeout period is set on the redis server, the client needs to perform connection timeout detection in time. However, the configuration provided in the current spring-boot-autoconfigure is incomplete:

• org.springframework.boot.autoconfigure.data.redis.JedisConnectionConfiguration#jedisPoolConfig
• org.springframework.boot.autoconfigure.data.redis.RedisProperties.Pool

I expect to be able to inject the following configuration automatically

• org.apache.commons.pool2.impl.BaseObjectPoolConfig#durationBetweenEvictionRuns
• org.apache.commons.pool2.impl.BaseObjectPoolConfig#minEvictableIdleDuration
• org.apache.commons.pool2.impl.BaseObjectPoolConfig#numTestsPerEvictionRun
• org.apache.commons.pool2.impl.BaseObjectPoolConfig#testOnBorrow

In addition to durationBetweenEvictionRuns, three other key configuration does not support automatic injection.
Now that you can inject durationBetweenEvictionRuns automatically, then the other three configuration is also should be automatic injection

[scottfrederick]

@yhmingyue Thanks for getting in touch. Can you clarify what you mean when you say "inject the following configuration automatically" and "automatic injection"? Are you asking for configuration properties under spring.data.redis.jedis.pool that can be used to configure addition pooling properties?

You can currently create a JedisClientConfigurationBuilderCustomizer to customize the configuration that is provided by Spring Boot auto-configuration. For your use case, you would write a new class in your project that looks like this:

@Configuration
public class RedisConfiguration {
	@Bean
	JedisClientConfigurationBuilderCustomizer customizer() {
		return (builder) -> {
			GenericObjectPoolConfig poolConfig = builder.build().getPoolConfig().orElseGet(GenericObjectPoolConfig::new);
			poolConfig.setTimeBetweenEvictionRuns(...);
			poolConfig.setMinEvictableIdleTime(...);
			poolConfig.setNumTestsPerEvictionRun(.);
			poolConfig.setTestOnBorrow(...);
			builder.usePooling().poolConfig(poolConfig);
		};
	}
}

Does that meet your needs?

[yhmingyue]

Thanks for your response, my solution is to write a new class in the project, as shown below：

@Configuration
public class JedisPoolConfiguration implements JedisClientConfigurationBuilderCustomizer {

    @Autowired
    private RedisProperties properties;

    private static final boolean COMMONS_POOL2_AVAILABLE = ClassUtils.isPresent("org.apache.commons.pool2.ObjectPool",
            JedisPoolConfiguration.class.getClassLoader());

    @Value(value = "${spring.redis.jedis.pool.minEvictableIdleTime:30M}")
    private Duration minEvictableIdleTime;
    
    @Value(value = "${spring.redis.jedis.pool.numTestsPerEvictionRun:3}")
    private int numTestsPerEvictionRun;

    @Value(value = "${spring.redis.jedis.pool.testOnBorrow:false}")
    private boolean testOnBorrow;

    @Override
    public void customize(JedisClientConfiguration.JedisClientConfigurationBuilder clientConfigurationBuilder) {
        RedisProperties.Pool pool = properties.getJedis().getPool();
        if (isPoolEnabled(pool)) {
            applyPooling(pool, clientConfigurationBuilder);
        }
    }

    private void applyPooling(RedisProperties.Pool pool,
                              JedisClientConfiguration.JedisClientConfigurationBuilder builder) {
        builder.usePooling().poolConfig(jedisPoolConfig(pool));
    }

    private JedisPoolConfig jedisPoolConfig(RedisProperties.Pool pool) {
        JedisPoolConfig config = new JedisPoolConfig();
        config.setMaxTotal(pool.getMaxActive());
        config.setMaxIdle(pool.getMaxIdle());
        config.setMinIdle(pool.getMinIdle());
        if (pool.getTimeBetweenEvictionRuns() != null) {
            config.setTimeBetweenEvictionRuns(pool.getTimeBetweenEvictionRuns());
        }
        if (pool.getMaxWait() != null) {
            config.setMaxWait(pool.getMaxWait());
        }
        if (minEvictableIdleTime != null){
            config.setMinEvictableIdleTime(minEvictableIdleTime);
        }
        config.setNumTestsPerEvictionRun(numTestsPerEvictionRun);
        config.setTestOnBorrow(testOnBorrow);
        return config;
    }

    protected boolean isPoolEnabled(RedisProperties.Pool pool) {
        Boolean enabled = pool.getEnabled();
        return (enabled != null) ? enabled : COMMONS_POOL2_AVAILABLE;
    }
}

But I would expect the auto-configuration of spring-boot to support the other three parameters by default。
Because these other three configurations are critical when it comes to connection pool timeout detection。

[scottfrederick]

There are a few dozen parameters in the Apache Commons Pool configuration classes used by Jedis and several other types of connections that Spring Boot auto-configures. Spring Boot typically provides properties for a few of the most commonly used parameters and relies on customizers as shown in my example above to let users configure the rest. Your sample code could be simplified to reduce the amount of code copied from Spring Boot and only set the additional parameters you care about.

That said, most of the parameters that you mention are also treated specially in the Jedis driver, with defaults provided. So I can see the case that it is important that these parameters are set correctly.

I'm marking this issue for team attention so other core team members can give their thoughts on these pooling properties.

[wilkinsona]

On the face of it, providing spring.redis.jedis.pool.* properties for the settings the Jedis deems to be important sounds like a reasonable balance to me. I think it would require us to create a Jedis-specific Pool class for configuration property binding as I think we'd want different defaults with Jedis and Lettuce so that we align with each pool's own defaults.