Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Lock TTL configuration using DefaultRedisCacheWriter #2300

Closed
artrodkin opened this issue Apr 11, 2022 · 2 comments
Closed

Support Lock TTL configuration using DefaultRedisCacheWriter #2300

artrodkin opened this issue Apr 11, 2022 · 2 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@artrodkin
Copy link

artrodkin commented Apr 11, 2022

We ran into the situation that was referenced in DATAREDIS-1052

We had an exception during the attempt to unlock:

ExceptionCause="redis.clients.jedis.exceptions.JedisConnectionException: Attempting to read from a broken connection" ExceptionStack="org.springframework.data.redis.RedisConnectionFailureException: Attempting to read from a broken connection; nested exception is redis.clients.jedis.exceptions.JedisConnectionException: Attempting to read from a broken connection
	at org.springframework.data.redis.connection.jedis.JedisExceptionConverter.convert(JedisExceptionConverter.java:65)
	at org.springframework.data.redis.connection.jedis.JedisExceptionConverter.convert(JedisExceptionConverter.java:42)
	at org.springframework.data.redis.PassThroughExceptionTranslationStrategy.translate(PassThroughExceptionTranslationStrategy.java:44)
	at org.springframework.data.redis.FallbackExceptionTranslationStrategy.translate(FallbackExceptionTranslationStrategy.java:42)
	at org.springframework.data.redis.connection.jedis.JedisConnection.convertJedisAccessException(JedisConnection.java:135)
	at org.springframework.data.redis.connection.jedis.JedisKeyCommands.del(JedisKeyCommands.java:122)
	at org.springframework.data.redis.connection.DefaultedRedisConnection.del(DefaultedRedisConnection.java:82)
	at org.springframework.data.redis.cache.DefaultRedisCacheWriter.doUnlock(DefaultRedisCacheWriter.java:228)


After this , all threads trying to access the key were stuck waiting:

java.lang.Thread.State: TIMED_WAITING (sleeping)
	at java.lang.Thread.sleep(Native Method)
	at org.springframework.data.redis.cache.DefaultRedisCacheWriter.checkAndPotentiallyWaitUntilUnlocked(DefaultRedisCacheWriter.java:274)
	at org.springframework.data.redis.cache.DefaultRedisCacheWriter.execute(DefaultRedisCacheWriter.java:247)
	at org.springframework.data.redis.cache.DefaultRedisCacheWriter.get(DefaultRedisCacheWriter.java:110)
	at org.springframework.data.redis.cache.RedisCache.lookup(RedisCache.java:88)

And there was no client to release this lock.

We were wondering if it would be possible to add the ability to send the expiration time in the SET NX command when this lock is created, so that system could automatically recover from this situation.

Thanks
Art

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 11, 2022
@mp911de mp911de added the type: bug A general bug label Apr 19, 2022
@mp911de
Copy link
Member

mp911de commented Apr 20, 2022

The issue is caused by the lock entry not being cleaned up. In this case, the cleanup failed because the connection became unusable. There are a couple of ways out here:

  1. Setting a TTL
  2. Retrying the cleanup

While we could retry the cleanup in case the connection is disconnected, the cleanup could still fail in case the Redis server went down.

I think that we should extend our RedisCacheWriter creation to allow specifying a lock TTL. We have already a few configuration properties such as BatchStrategy and SleepTimeout that we could pull together into another configuration object and add the lock TTL as a third configuration option so different caches can use different lock timeouts.

@mp911de mp911de added type: enhancement A general enhancement and removed type: bug A general bug status: waiting-for-triage An issue we've not yet triaged labels Apr 20, 2022
@mp911de mp911de changed the title DefaultRedisCacheWriter can lead to infinite lock Locking DefaultRedisCacheWriter can lead to infinite lock if lock entry is not cleaned up Apr 20, 2022
@mp911de mp911de self-assigned this Apr 20, 2022
@mp911de mp911de changed the title Locking DefaultRedisCacheWriter can lead to infinite lock if lock entry is not cleaned up Support Lock TTL configuration using DefaultRedisCacheWriter Oct 11, 2022
@mp911de mp911de added the status: pending-design-work Needs design work before any code can be developed label Oct 11, 2022
@mp911de
Copy link
Member

mp911de commented Oct 11, 2022

We require a revision of our configuration support for DefaultRedisCacheWriter. The number of supported features is growing and adding another argument to the factory methods would harm its usability.

Likely, a configuration abstraction can help evolve the default implementation.

@mp911de mp911de removed the status: pending-design-work Needs design work before any code can be developed label Jun 7, 2023
@jxblum jxblum closed this as completed in a2f9a8b Jun 8, 2023
@mp911de mp911de added this to the 3.2 M1 (2023.1.0) milestone Jun 8, 2023
jxblum added a commit to jxblum/spring-data-redis that referenced this issue Jun 8, 2023
jxblum pushed a commit to jxblum/spring-data-redis that referenced this issue Jun 12, 2023
RedisCacheWriter now can now issue locks that expire using TtlFunction to prevent eternal locks.

Closes spring-projects#2300
Pull request: spring-projects#2597
jxblum added a commit to jxblum/spring-data-redis that referenced this issue Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants