JedisConnectionFactory failed to get connection after configuration auto update

[Jiabao-Sun]

Error Trace:

java.lang.IllegalStateException: JedisConnectionFactory was destroyed and cannot be used anymore## at org.springframework.util.Assert.state(Assert.java:76) ~[spring-core-5.3.23.jar!/:5.3.23]## at org.springframework.data.redis.connection.jedis.JedisConnectionFactory.assertInitialized(JedisConnectionFactory.java:971) ~[spring-data-redis-2.7.3.jar!/:2.7.3]## at org.springframework.data.redis.connection.jedis.JedisConnectionFactory.getConnection(JedisConnectionFactory.java:509) ~[spring-data-redis-2.7.3.jar!/:2.7.3]## at org.springframework.data.redis.core.RedisConnectionUtils.fetchConnection(RedisConnectionUtils.java:193) ~[spring-data-redis-2.7.3.jar!/:2.7.3]## at org.springframework.data.redis.core.RedisConnectionUtils.doGetConnection(RedisConnectionUtils.java:144)

We receive EnvironmentChangeEvent in the configuration hot update scenario.
The ConfigurationPropertiesRebinder will destroy and reinitialize JedisConnectionFactory.

In destroy method we set the destroyed flag as true.

But in initial method (afterPropertiesSet) we don't reset destroyed as false.

This will cause us to be unable to get a connection from the JedisConnectionFactory any more.

[jxblum]

@Jiabao-Sun - This logic in Spring Data Redis may actually be by design and it could be the ConfigurationPropertiesRebinder that is actually and technically incorrect here.

DISCLAIMER: I say "... may actually be by design" since this predates me. I have only recently taken over as the lead of Spring Data Redis and assumed the responsibilities for this Spring Data module. So, I am technically still learning about the reasons for our design decisions (in SD Redis) along with the underlying nuances of Redis itself (i.e. outside of a Spring context).

NOTE: FYI, I am was formerly the lead of all things Spring for Apache Geode (SDG, STDG, SSDG and SBDG), which has considerable overlap with Redis in terms of behavior and functionality. For instance, both are key/value stores but Apache Geode is a true IMDG unlike Redis. Anyway...

I observed these assertions (and specifically, this assertion) in the Spring Data Redis codebase, which also originates in the JedisConnectionFactory, and is called before acquiring new connections (Standalone, Clustered or Sentinel).

In fact, it may not be "safe" to reacquire connections from a "closed" factory (Jedis or Lettuce), which I suppose would highly depend on the Redis (client) driver specifically (using either Jedis or Lettuce), given there could be lingering, stale state. It really depends on the drivers, actually and technically.

The real question is, why is this ConfigurationPropertiesRebinder (of all things) responsible for destroying and then (re-initializing) a SD Redis, JedisConnectionFactory? This does not seem correct to me, or rather, this should not be responsibility of "configuration property binding".

In most case, with most stores (that don't offer some type of (connection) failover, particularly in a distributed, clustered topology arrangement), then the safest approach it to restart the (Spring) application, which will reinitialize the connection pool between a fresh JVM process and the Redis server(s).

[jxblum]

NOTE: Sorry about the same assertions reference(s) that you included in your issue ticket description.

Is this ConfigurationPropertiesRebinder class of your own Spring application's doing?

It does not originate in either Spring Data Redis or Spring Boot. I am curious where this class comes from?

[jxblum]

Thinking further on this...

You'd be better off "refreshing" (Javadoc) the Spring container (ApplicationContext) rather than trying to reinitialize a "named" (Redis) bean (such as the JedisConnectionFactory) in this manner, which arguably was never meat to be re-initialized in the first place, according to the current logic and design in SD Redis itself.

Of course, refreshing the Spring ApplicationContext refreshes the entire context and all Spring beans, which is more expensive than refreshing a single bean. But then, the SD Redis JedisConnectionFactory is not a trivial bean now is it.

Furthermore, you have control over declaring and constructing/initializing the RedisConnectionFactory in use here.

That is even a requirement when initializing your Spring Data Redis application in the first place. See ref doc in general, and using the JedisConnectionFactory in particular (here).

In other words, you must create a Spring managed bean of type RedisConnectionFactory, such as a new JedisConnectionFactory for your Spring application to even be able to connect to Redis servers.

In which case, you could extend the JedisConnectionFactory class and simply reset the destroyed field, reflectively (in afterPropertiesSet(). Though, I wouldn't even recommend this, since destroyed is an "internal" control variable that was not meant to be exposed in the API and set by the user (application) directly.

If you use this later approach, then you assume full responsibility of the extensions and behavioral override. This does not fall on Spring Data Redis since this is very application use case specific.

If you concerned about application refresh/restart times, then you can explore Spring Native Image support.

FYI, Spring is eventually doing to support CRaC, which is alternative to GraalVM Native, Images, and may be of interests to your application use case.