Support TTI expiration in Redis Cache implementation #2649

jxblum · 2023-07-21T01:35:29Z

See issue #2351.

See original pull request #2643.

We now support time-to-idle (TTI) expiration policies for cache reads. The TTI implementation is achieved with the use of the Redis GETEX command on Cache.get(key) operations as well as consistently using the same TTL configuration for all cache operations when TTI is enabled and TTL expiration has been configured, with the use of a TtlFunction or fixed Duration. Closes #2351 Original pull request: #2643

quaff · 2023-07-21T01:53:50Z

src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java


 		Assert.notNull(name, "Name must not be null");
 		Assert.notNull(key, "Key must not be null");

-		byte[] result = execute(name, connection -> connection.get(key));
+		byte[] result = shouldExpireWithin(ttl)
+			? execute(name, connection -> connection.getEx(key, Expiration.from(ttl)))


Should we fallback to get and expire if server doesn't support getex?

Thank you for the feedback, @quaff.

I spoke with @mp911de about this earlier today and we decided this feature will only be included from Spring Data Redis 3.2 and later, which is based on the latest Jedis and Lettuce drivers supporting most of the features in the newest Redis server releases.

In addition, it is an opt-in feature. See here and here, defaulting to the previous behavior.

which is based on the latest Jedis and Lettuce drivers supporting most of the features in the newest Redis server releases.

I mean server not client doesn't support getex, it's a new command since redis 6.2.0, upgrading server is not convenient as client, many server still using 4.x and 5.x, especially service provided by private cloud.

If the server does not support getex then you cannot use Time to Idle.

We could fallback to get and expire.

@quaff - I understood what you meant between client and server versions.

@christophstrobl and I are meeting this week on the topic of caching in SD Redis and using TTI-like expiration. Let me touch base with Christoph on this whether or not we will take into consideration Redis server version.

mp911de

Great work, I left a few comments we should consider regarding server versions and naming.

mp911de · 2023-07-21T13:18:35Z

src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java

@@ -47,9 +47,11 @@
 public class RedisCacheConfiguration {

 	protected static final boolean DEFAULT_CACHE_NULL_VALUES = true;
+	protected static final boolean DEFAULT_ENABLE_TTI_EXPIRATION = false;


Let's use TIME_TO_IDLE instead of TTI for more expressivity.

Will do.

Ironically, I initially spelled it out in favor of being explicit, but ultimately opted to match our use of the "TTL" abbreviation. It is definitely easy to overlook and possibly confuse "TTI" with "TTL" on a brief glance.

mp911de · 2023-07-21T13:19:35Z

src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java

 	protected static final boolean DEFAULT_USE_PREFIX = true;
 	protected static final boolean DO_NOT_CACHE_NULL_VALUES = false;
 	protected static final boolean DO_NOT_USE_PREFIX = false;
+	protected static final boolean ENABLE_IDLE_TIME_EXPIRATION = true;


@quaff has a good point regarding server versions. We cannot assume the application runs on the latest Redis version. If we'd enabled this for everyone, then all users with older Redis versions suddenly see failures on their target environments unless they reconfigure the cache.

I did similar work before, like this:

private volatile boolean getexSupported = true; @Override public Object getWithTti(String key, int timeToIdle, TimeUnit timeUnit) { BoundValueOperations operations = redisTemplate.boundValueOps(key); try { if (getexSupported) return timeToIdle > 0 ? operations.getAndExpire(timeToIdle, timeUnit) : operations.getAndPersist(); // fallback to get and expire Object result = operations.get(); if (result != null && timeToIdle > 0) operations.expire(timeToIdle, timeUnit); return result; } catch (RuntimeException e) { log.error(e.getMessage(), e); if (e.getCause() instanceof RedisCommandExecutionException) { getexSupported = false; // server.version < 6.2 return getWithTti(key, timeToIdle, timeUnit); } throw e; } }

Please note, the ENABLE_IDLE_TIME_EXPIRATION constant was meant to express the preferred use of time-to-idle (TTI) expiration when "explicitly enabled" and is NOT a flag defaulting TTI to true. The "default" value for TTI is expressed constant DEFAULT_ENABLE_TTI_EXPIRATION, here.

I prefer to use "named" constants rather than "true", especially when calling constructors or methods accepting (multiple) boolean-typed arguments in order to make it clear "what" is actually being enabled. For instance.

This similar to the existing constants for not caching null values (disableCachingNullValues(); here) or not using a cache key prefix, (disableKeyPrefix(); here).

However, I realize the constant name (ENABLE_IDLE_TIME_EXPIRATION ) was a poor choice of words and I have changed this to USE_TIME_TO_IDLE_EXPIRATION. This is now, not unlike the constants for caching null values or using a cache key prefix.

Keep in mind, this constant (now USE_TIME_TO_IDLE_EXPIRATION) is only ever used when TTI is enabled by calling the enableTtiExpiration() method on the RedisCacheConfiguration instance, here. This makes it very explicit as to what is being set (TTI) and how (use (enable) TTI).

@quaff - FYI, if @christophstrobl and I decide it makes sense to handle Redis server version for a (Redis) Cache.get(key), using either GETEX or a GET with EXPIRE (although, I am not certain SD Redis takes Redis server version into consideration anywhere else, though), then I would envision something a bit more robust, such as by (proactively) inspecting the Redis server version on construction of the DefaultRedisCacheWriter using the INFO command from RedisServerCommands given a RedisConnection (retrieved from the factory).

mp911de · 2023-07-21T13:22:33Z

src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java

 				CacheKeyPrefix.simple(),
 				SerializationPair.fromSerializer(RedisSerializer.string()),
 				SerializationPair.fromSerializer(RedisSerializer.java(classLoader)),
 				conversionService);
 	}

 	private final boolean cacheNullValues;
+	private final boolean enableTtiExpiration;


Let's expand on the name to enableTimeToIdle instead of Tti.

Done.

I also renamed isTtiExpirationEnabled() to isTimeToIdleEnabled() and enableTtiExpiration() to enableTimeToIdle().

mp911de · 2023-07-21T13:23:58Z

src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java

+	 * expiration are used consistently across the application, then in effect, an application can achieve
+	 * {@literal TTI} expiration-like behavior.
+	 *
+	 * @return this {@link RedisCacheConfiguration}.


Nit: @since 3.2

Good catch!

mp911de · 2023-07-21T13:24:08Z

src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java

+	 * Defaults to {@literal false}.
+	 * @see <a href="https://redis.io/commands/getex/">GETEX</a>
+	 */
+	public boolean isTtiExpirationEnabled() {


Nit: @since 3.2

mp911de · 2023-07-21T13:26:42Z

src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterUnitTests.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2017-2023 the original author or authors.


Nit: Copyright starts at 2023 (inception of the file).

mp911de · 2023-07-21T13:27:27Z

src/test/java/org/springframework/data/redis/cache/RedisCacheWriterUnitTests.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2017-2023 the original author or authors.


Nit: Copyright starts at 2023.

mp911de · 2023-07-21T13:27:44Z

src/test/java/org/springframework/data/redis/cache/RedisCacheWriterUnitTests.java

+ *
+ * @author John Blum
+ */
+public class RedisCacheWriterUnitTests {


Nit: JUnit 5 test classes can be package-private.

mp911de · 2023-07-21T13:28:01Z

src/test/java/org/springframework/data/redis/core/types/ExpirationUnitTests.java

@@ -53,4 +56,38 @@ void fromMinutes() {
 		assertThat(expiration.getExpirationTime()).isEqualTo(5L * 60);
 		assertThat(expiration.getTimeUnit()).isEqualTo(TimeUnit.SECONDS);
 	}
+
+	@Test
+	public void equalValuedExpirationsAreEqual() {


Nit: Test methods can be package-private.

mp911de · 2023-07-21T13:28:07Z

src/test/java/org/springframework/data/redis/core/types/ExpirationUnitTests.java

 import java.util.concurrent.TimeUnit;

 import org.junit.jupiter.api.Test;

 /**
+ * Unit tests for {@link Expiration}.


christophstrobl · 2023-07-25T11:22:23Z

src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java

@@ -15,10 +15,10 @@
 */
 package org.springframework.data.redis.cache;

-import static org.assertj.core.api.Assertions.*;


The import changes do not seem in line with the import order defined in spring-data-build.

Closes #2351

jxblum · 2023-07-26T00:19:33Z

I am not sure why, but the GitHub PR did not pick up my latest set of changes, a polishing commit based on the review feedback above.

… is available. The Redis GETEX command is only available when the Redis server version is at least 6.2.0. Closes #2351

…Spring Data Redis Cache implementation. Edit Javadoc in RedisCacheManager. Closes #2351

jxblum added 2 commits July 20, 2023 15:43

Prepare issue branch for 2351.

9655db8

jxblum requested review from christophstrobl and mp911de July 21, 2023 01:35

This was referenced Jul 21, 2023

Add support for GETEX in Spring Redis Cache #2643

Closed

Add support for GETEX in Spring Redis Cache #2351

Closed

quaff reviewed Jul 21, 2023

View reviewed changes

mp911de reviewed Jul 21, 2023

View reviewed changes

mp911de assigned christophstrobl Jul 21, 2023

christophstrobl reviewed Jul 25, 2023

View reviewed changes

Polish new TTI expiration functionality from review feedback.

20d8ea6

Closes #2351

jxblum added 2 commits July 26, 2023 12:46

Enable Redis Cache TTI expiration integration tests iff GETEX command…

1da9bf6

… is available. The Redis GETEX command is only available when the Redis server version is at least 6.2.0. Closes #2351

Edit ref doc to include information on TTL/TTI expiration when using …

2b06993

…Spring Data Redis Cache implementation. Edit Javadoc in RedisCacheManager. Closes #2351

jxblum added this to the 3.2 M2 (2023.1.0) milestone Aug 1, 2023

jxblum added the in: cache RedisCache and CacheManager label Aug 1, 2023

jxblum closed this Aug 1, 2023

jxblum deleted the issue/2351 branch October 12, 2023 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support TTI expiration in Redis Cache implementation #2649

Support TTI expiration in Redis Cache implementation #2649

jxblum commented Jul 21, 2023 •

edited

Loading

quaff Jul 21, 2023

jxblum Jul 21, 2023

quaff Jul 21, 2023

mp911de Jul 21, 2023 •

edited

Loading

quaff Jul 24, 2023

jxblum Jul 25, 2023 •

edited

Loading

mp911de left a comment

mp911de Jul 21, 2023

jxblum Jul 25, 2023 •

edited

Loading

mp911de Jul 21, 2023

quaff Jul 24, 2023

jxblum Jul 25, 2023 •

edited

Loading

jxblum Jul 25, 2023 •

edited

Loading

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

jxblum Jul 25, 2023

mp911de Jul 21, 2023

christophstrobl Jul 25, 2023

jxblum commented Jul 26, 2023

		@@ -0,0 +1,99 @@
		/*
		* Copyright 2017-2023 the original author or authors.

		@@ -0,0 +1,56 @@
		/*
		* Copyright 2017-2023 the original author or authors.

Support TTI expiration in Redis Cache implementation #2649

Support TTI expiration in Redis Cache implementation #2649

Conversation

jxblum commented Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mp911de Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

mp911de left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

jxblum Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum commented Jul 26, 2023

jxblum commented Jul 21, 2023 •

edited

Loading

mp911de Jul 21, 2023 •

edited

Loading

jxblum Jul 25, 2023 •

edited

Loading

jxblum Jul 25, 2023 •

edited

Loading

jxblum Jul 25, 2023 •

edited

Loading

jxblum Jul 25, 2023 •

edited

Loading