diff --git a/docs/src/docs/asciidoc/index.adoc b/docs/src/docs/asciidoc/index.adoc index 233e211..e335da4 100644 --- a/docs/src/docs/asciidoc/index.adoc +++ b/docs/src/docs/asciidoc/index.adoc @@ -301,6 +301,9 @@ For example: SADD spring:session:expirations: EXPIRE spring:session:expirations: 1800 +The background task will then use these mappings to explicitly request each key. +By accessing they key, rather than deleting it, we ensure that Redis deletes the key for us only if the TTL is expired. + The Redis expiration is still placed on each key to ensure that if the server is down when the session expires, it is still cleaned up. [[api-redisoperationssessionrepository-writes]] diff --git a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java index d67c381..1e30452 100644 --- a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java @@ -104,8 +104,14 @@ import org.springframework.util.Assert; * EXPIRE spring:session:expirations: 1800 * } * + *

+ * The background task will then use these mappings to explicitly request each key. + * By accessing they key, rather than deleting it, we ensure that Redis deletes the key for us only if the TTL is expired. + *

+ *

* The Redis expiration is still placed on each key to ensure that if the server * is down when the session expires, it is still cleaned up. + *

* * @since 1.0 * diff --git a/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java b/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java index 1f7b8af..c872703 100644 --- a/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java +++ b/spring-session/src/main/java/org/springframework/session/data/redis/RedisSessionExpirationPolicy.java @@ -35,7 +35,7 @@ import org.springframework.session.data.redis.RedisOperationsSessionRepository.R * order to ensure expired session events are processed in a timely fashion the * expiration (rounded to the nearest minute) is mapped to all the sessions that * expire at that time. Whenever {@link #cleanExpiredSessions()} is invoked, the - * sessions for the previous minute are then deleted explicitly. + * sessions for the previous minute are then accessed to ensure they are deleted if expired. * * In some instances the {@link #cleanExpiredSessions()} method may not be not * invoked for a specific time. For example, this may happen when a server is @@ -108,18 +108,21 @@ final class RedisSessionExpirationPolicy { String expirationKey = getExpirationKey(prevMin); Set sessionsToExpire = expirationRedisOperations.boundSetOps(expirationKey).members(); - Set keysToDelete = new HashSet(sessionsToExpire.size() + 1); - keysToDelete.add(expirationKey); + touch(expirationKey); for(String session : sessionsToExpire) { String sessionKey = getSessionKey(session); - keysToDelete.add(sessionKey); + touch(sessionKey); } + } - sessionRedisOperations.delete(keysToDelete); - - if(logger.isDebugEnabled()) { - logger.debug("The following expired Sessions were deleted " + keysToDelete); - } + /** + * By trying to access the session we only trigger a deletion if it the TTL is expired. This is done to handle + * https://github.com/spring-projects/spring-session/issues/93 + * + * @param key + */ + private void touch(String key) { + sessionRedisOperations.hasKey(key); } private long roundUpToNextMinute(long timeInMs, int inactiveIntervalInSec) { diff --git a/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java b/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java index d736f4a..da0c75d 100644 --- a/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java +++ b/spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java @@ -47,8 +47,6 @@ public class RedisOperationsSessionRepositoryTests { BoundSetOperations boundSetOperations; @Captor ArgumentCaptor> delta; - @Captor - ArgumentCaptor> keys; private RedisOperationsSessionRepository redisRepository; @@ -237,19 +235,17 @@ public class RedisOperationsSessionRepositoryTests { String expiredId = "expired-id"; when(redisOperations.boundHashOps(getKey(expiredId))).thenReturn(boundHashOperations); when(redisOperations.boundSetOps(anyString())).thenReturn(boundSetOperations); + Set expiredIds = new HashSet(Arrays.asList("expired-key1","expired-key2")); when(boundSetOperations.members()).thenReturn(expiredIds); redisRepository.cleanupExpiredSessions(); - verify(redisOperations).delete(keys.capture()); - for(String id : expiredIds) { String expiredKey = RedisOperationsSessionRepository.BOUNDED_HASH_KEY_PREFIX + id; - assertThat(keys.getValue()).contains(expiredKey); + // https://github.com/spring-projects/spring-session/issues/93 + verify(redisOperations).hasKey(expiredKey); } - // the key that maps the expiration time to the expired ids - assertThat(keys.getValue().size()).isEqualTo(expiredIds.size() + 1); } @SuppressWarnings("rawtypes")