Skip to content

Commit b4583c4

Browse files
vpavicvpavic
committed
Simplify expired session cleanup jobs
At present, RedisIndexedHttpSessionConfiguration and JdbcHttpSessionConfiguration include [at]EnableScheduling annotated inner configuration classes that configure expired session cleanup jobs. This approach silently opts in users into general purpose task scheduling support provided by Spring Framework, which isn't something a library should do. Ideally, session cleanup jobs should only require a single thread dedicated to their execution and also one that doesn't compete for resources with general purpose task scheduling. This commit updates RedisIndexedSessionRepository and JdbcIndexedSessionRepository to have them manage their own ThreadPoolTaskScheduler for purposes of running expired session cleanup jobs. Closes gh-2136
1 parent cb6f5c9 commit b4583c4

File tree

12 files changed

+163
-69
lines changed

12 files changed

+163
-69
lines changed

‎spring-session-data-redis/spring-session-data-redis.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ description = "Spring Session Redis implementation"
44

55
dependencies {
66
api project(':spring-session-core')
7+
api "jakarta.annotation:jakarta.annotation-api"
78
api ("org.springframework.data:spring-data-redis") {
89
exclude group: "org.slf4j", module: 'jcl-over-slf4j'
910
}

‎spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
import java.util.Map;
2525
import java.util.Set;
2626

27+
import jakarta.annotation.PostConstruct;
28+
import jakarta.annotation.PreDestroy;
29+
2730
import org.apache.commons.logging.Log;
2831
import org.apache.commons.logging.LogFactory;
2932

@@ -38,6 +41,10 @@
3841
import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer;
3942
import org.springframework.data.redis.serializer.RedisSerializer;
4043
import org.springframework.data.redis.util.ByteUtils;
44+
import org.springframework.scheduling.annotation.Scheduled;
45+
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
46+
import org.springframework.scheduling.support.CronExpression;
47+
import org.springframework.scheduling.support.CronTrigger;
4148
import org.springframework.session.DelegatingIndexResolver;
4249
import org.springframework.session.FindByIndexNameSessionRepository;
4350
import org.springframework.session.FlushMode;
@@ -255,6 +262,11 @@ public class RedisIndexedSessionRepository
255262

256263
private static final String SPRING_SECURITY_CONTEXT = "SPRING_SECURITY_CONTEXT";
257264

265+
/**
266+
* The default cron expression used for expired session cleanup job.
267+
*/
268+
public static final String DEFAULT_CLEANUP_CRON = "0 * * * * *";
269+
258270
/**
259271
* The default Redis database used by Spring Session.
260272
*/
@@ -309,6 +321,10 @@ public class RedisIndexedSessionRepository
309321

310322
private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE;
311323

324+
private String cleanupCron = DEFAULT_CLEANUP_CRON;
325+
326+
private ThreadPoolTaskScheduler taskScheduler;
327+
312328
/**
313329
* Creates a new instance. For an example, refer to the class level javadoc.
314330
* @param sessionRedisOperations the {@link RedisOperations} to use for managing the
@@ -322,6 +338,28 @@ public RedisIndexedSessionRepository(RedisOperations<String, Object> sessionRedi
322338
configureSessionChannels();
323339
}
324340

341+
private static ThreadPoolTaskScheduler createTaskScheduler() {
342+
ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler();
343+
taskScheduler.setThreadNamePrefix("spring-session-");
344+
return taskScheduler;
345+
}
346+
347+
@PostConstruct
348+
void init() {
349+
if (!Scheduled.CRON_DISABLED.equals(this.cleanupCron)) {
350+
this.taskScheduler = createTaskScheduler();
351+
this.taskScheduler.initialize();
352+
this.taskScheduler.schedule(this::cleanUpExpiredSessions, new CronTrigger(this.cleanupCron));
353+
}
354+
}
355+
356+
@PreDestroy
357+
void destroy() {
358+
if (this.taskScheduler != null) {
359+
this.taskScheduler.destroy();
360+
}
361+
}
362+
325363
/**
326364
* Sets the {@link ApplicationEventPublisher} that is used to publish
327365
* {@link SessionDestroyedEvent}. The default is to not publish a
@@ -382,6 +420,21 @@ public void setSaveMode(SaveMode saveMode) {
382420
this.saveMode = saveMode;
383421
}
384422

423+
/**
424+
* Set the cleanup cron expression.
425+
* @param cleanupCron the cleanup cron expression
426+
* @since 3.0.0
427+
* @see CronExpression
428+
* @see Scheduled#CRON_DISABLED
429+
*/
430+
public void setCleanupCron(String cleanupCron) {
431+
Assert.notNull(cleanupCron, "cleanupCron must not be null");
432+
if (!Scheduled.CRON_DISABLED.equals(cleanupCron)) {
433+
Assert.isTrue(CronExpression.isValidExpression(cleanupCron), "cleanupCron must be valid");
434+
}
435+
this.cleanupCron = cleanupCron;
436+
}
437+
385438
/**
386439
* Sets the database index to use. Defaults to {@link #DEFAULT_DATABASE}.
387440
* @param database the database index to use
@@ -420,7 +473,7 @@ public void save(RedisSession session) {
420473
}
421474
}
422475

423-
public void cleanupExpiredSessions() {
476+
public void cleanUpExpiredSessions() {
424477
this.expirationPolicy.cleanExpiredSessions();
425478
}
426479

‎spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/EnableRedisIndexedHttpSession.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,6 @@
108108
* The cron expression for expired session cleanup job. By default runs every minute.
109109
* @return the session cleanup cron expression
110110
*/
111-
String cleanupCron() default RedisIndexedHttpSessionConfiguration.DEFAULT_CLEANUP_CRON;
111+
String cleanupCron() default RedisIndexedSessionRepository.DEFAULT_CLEANUP_CRON;
112112

113113
}

‎spring-session-data-redis/src/main/java/org/springframework/session/data/redis/config/annotation/web/http/RedisIndexedHttpSessionConfiguration.java‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@
4141
import org.springframework.data.redis.listener.ChannelTopic;
4242
import org.springframework.data.redis.listener.PatternTopic;
4343
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
44-
import org.springframework.scheduling.annotation.EnableScheduling;
45-
import org.springframework.scheduling.annotation.SchedulingConfigurer;
46-
import org.springframework.scheduling.config.ScheduledTaskRegistrar;
4744
import org.springframework.session.IndexResolver;
4845
import org.springframework.session.Session;
4946
import org.springframework.session.data.redis.RedisIndexedSessionRepository;
@@ -68,9 +65,7 @@ public class RedisIndexedHttpSessionConfiguration
6865
extends AbstractRedisHttpSessionConfiguration<RedisIndexedSessionRepository>
6966
implements EmbeddedValueResolverAware, ImportAware {
7067

71-
static final String DEFAULT_CLEANUP_CRON = "0 * * * * *";
72-
73-
private String cleanupCron = DEFAULT_CLEANUP_CRON;
68+
private String cleanupCron = RedisIndexedSessionRepository.DEFAULT_CLEANUP_CRON;
7469

7570
private ConfigureRedisAction configureRedisAction = new ConfigureNotifyKeyspaceEventsAction();
7671

@@ -102,6 +97,7 @@ public RedisIndexedSessionRepository sessionRepository() {
10297
}
10398
sessionRepository.setFlushMode(getFlushMode());
10499
sessionRepository.setSaveMode(getSaveMode());
100+
sessionRepository.setCleanupCron(this.cleanupCron);
105101
int database = resolveDatabase();
106102
sessionRepository.setDatabase(database);
107103
getSessionRepositoryCustomizers()
@@ -247,25 +243,4 @@ public void afterPropertiesSet() {
247243

248244
}
249245

250-
/**
251-
* Configuration of scheduled job for cleaning up expired sessions.
252-
*/
253-
@EnableScheduling
254-
@Configuration(proxyBeanMethods = false)
255-
class SessionCleanupConfiguration implements SchedulingConfigurer {
256-
257-
private final RedisIndexedSessionRepository sessionRepository;
258-
259-
SessionCleanupConfiguration(RedisIndexedSessionRepository sessionRepository) {
260-
this.sessionRepository = sessionRepository;
261-
}
262-
263-
@Override
264-
public void configureTasks(ScheduledTaskRegistrar taskRegistrar) {
265-
taskRegistrar.addCronTask(this.sessionRepository::cleanupExpiredSessions,
266-
RedisIndexedHttpSessionConfiguration.this.cleanupCron);
267-
}
268-
269-
}
270-
271246
}

‎spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.springframework.data.redis.core.RedisOperations;
4545
import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer;
4646
import org.springframework.data.redis.serializer.RedisSerializer;
47+
import org.springframework.scheduling.annotation.Scheduled;
4748
import org.springframework.session.FindByIndexNameSessionRepository;
4849
import org.springframework.session.FlushMode;
4950
import org.springframework.session.MapSession;
@@ -451,7 +452,7 @@ void cleanupExpiredSessions() {
451452
Set<Object> expiredIds = new HashSet<>(Arrays.asList("expired-key1", "expired-key2"));
452453
given(this.boundSetOperations.members()).willReturn(expiredIds);
453454

454-
this.redisRepository.cleanupExpiredSessions();
455+
this.redisRepository.cleanUpExpiredSessions();
455456

456457
for (Object id : expiredIds) {
457458
String expiredKey = "spring:session:sessions:" + id;
@@ -744,6 +745,25 @@ void setFlushModeNull() {
744745
.withMessage("flushMode cannot be null");
745746
}
746747

748+
@Test
749+
void setCleanupCronNull() {
750+
assertThatIllegalArgumentException().isThrownBy(() -> this.redisRepository.setCleanupCron(null))
751+
.withMessage("cleanupCron must not be null");
752+
}
753+
754+
@Test
755+
void setCleanupCronInvalid() {
756+
assertThatIllegalArgumentException().isThrownBy(() -> this.redisRepository.setCleanupCron("test"))
757+
.withMessage("cleanupCron must be valid");
758+
}
759+
760+
@Test
761+
void setCleanupCronDisabled() {
762+
this.redisRepository.setCleanupCron(Scheduled.CRON_DISABLED);
763+
this.redisRepository.init();
764+
assertThat(this.redisRepository).extracting("taskScheduler").isNull();
765+
}
766+
747767
@Test
748768
void changeRedisNamespace() {
749769
String namespace = "foo:bar";

‎spring-session-data-redis/src/test/java/org/springframework/session/data/redis/config/annotation/web/http/RedisIndexedHttpSessionConfigurationTests.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,8 @@ void setCustomFlushImmediately() {
117117
void customCleanupCronAnnotation() {
118118
registerAndRefresh(RedisConfig.class, CustomCleanupCronExpressionAnnotationConfiguration.class);
119119

120-
RedisIndexedHttpSessionConfiguration configuration = this.context
121-
.getBean(RedisIndexedHttpSessionConfiguration.class);
122-
assertThat(configuration).isNotNull();
123-
assertThat(ReflectionTestUtils.getField(configuration, "cleanupCron")).isEqualTo(CLEANUP_CRON_EXPRESSION);
120+
RedisIndexedSessionRepository sessionRepository = this.context.getBean(RedisIndexedSessionRepository.class);
121+
assertThat(sessionRepository).extracting("cleanupCron").isEqualTo(CLEANUP_CRON_EXPRESSION);
124122
}
125123

126124
@Test

‎spring-session-jdbc/spring-session-jdbc.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ description = "Spring Session and Spring JDBC integration"
44

55
dependencies {
66
api project(':spring-session-core')
7+
api "jakarta.annotation:jakarta.annotation-api"
78
api "org.springframework:spring-context"
89
api "org.springframework:spring-jdbc"
910

‎spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
import java.util.function.Supplier;
3232
import java.util.stream.Collectors;
3333

34+
import jakarta.annotation.PostConstruct;
35+
import jakarta.annotation.PreDestroy;
36+
3437
import org.apache.commons.logging.Log;
3538
import org.apache.commons.logging.LogFactory;
3639

@@ -48,6 +51,10 @@
4851
import org.springframework.jdbc.support.lob.DefaultLobHandler;
4952
import org.springframework.jdbc.support.lob.LobCreator;
5053
import org.springframework.jdbc.support.lob.LobHandler;
54+
import org.springframework.scheduling.annotation.Scheduled;
55+
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
56+
import org.springframework.scheduling.support.CronExpression;
57+
import org.springframework.scheduling.support.CronTrigger;
5158
import org.springframework.session.DelegatingIndexResolver;
5259
import org.springframework.session.FindByIndexNameSessionRepository;
5360
import org.springframework.session.FlushMode;
@@ -138,6 +145,11 @@ public class JdbcIndexedSessionRepository
138145
*/
139146
public static final String DEFAULT_TABLE_NAME = "SPRING_SESSION";
140147

148+
/**
149+
* The default cron expression used for expired session cleanup job.
150+
*/
151+
public static final String DEFAULT_CLEANUP_CRON = "0 * * * * *";
152+
141153
private static final String SPRING_SECURITY_CONTEXT = "SPRING_SECURITY_CONTEXT";
142154

143155
private static final String CREATE_SESSION_QUERY = """
@@ -241,6 +253,10 @@ public class JdbcIndexedSessionRepository
241253

242254
private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE;
243255

256+
private String cleanupCron = DEFAULT_CLEANUP_CRON;
257+
258+
private ThreadPoolTaskScheduler taskScheduler;
259+
244260
/**
245261
* Create a new {@link JdbcIndexedSessionRepository} instance which uses the provided
246262
* {@link JdbcOperations} and {@link TransactionOperations} to manage sessions.
@@ -255,6 +271,28 @@ public JdbcIndexedSessionRepository(JdbcOperations jdbcOperations, TransactionOp
255271
prepareQueries();
256272
}
257273

274+
private static ThreadPoolTaskScheduler createTaskScheduler() {
275+
ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler();
276+
taskScheduler.setThreadNamePrefix("spring-session-");
277+
return taskScheduler;
278+
}
279+
280+
@PostConstruct
281+
void init() {
282+
if (!Scheduled.CRON_DISABLED.equals(this.cleanupCron)) {
283+
this.taskScheduler = createTaskScheduler();
284+
this.taskScheduler.initialize();
285+
this.taskScheduler.schedule(this::cleanUpExpiredSessions, new CronTrigger(this.cleanupCron));
286+
}
287+
}
288+
289+
@PreDestroy
290+
void destroy() {
291+
if (this.taskScheduler != null) {
292+
this.taskScheduler.destroy();
293+
}
294+
}
295+
258296
/**
259297
* Set the name of database table used to store sessions.
260298
* @param tableName the database table name
@@ -397,6 +435,21 @@ public void setSaveMode(SaveMode saveMode) {
397435
this.saveMode = saveMode;
398436
}
399437

438+
/**
439+
* Set the cleanup cron expression.
440+
* @param cleanupCron the cleanup cron expression
441+
* @since 3.0.0
442+
* @see CronExpression
443+
* @see Scheduled#CRON_DISABLED
444+
*/
445+
public void setCleanupCron(String cleanupCron) {
446+
Assert.notNull(cleanupCron, "cleanupCron must not be null");
447+
if (!Scheduled.CRON_DISABLED.equals(cleanupCron)) {
448+
Assert.isTrue(CronExpression.isValidExpression(cleanupCron), "cleanupCron must be valid");
449+
}
450+
this.cleanupCron = cleanupCron;
451+
}
452+
400453
@Override
401454
public JdbcSession createSession() {
402455
MapSession delegate = new MapSession();

‎spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/EnableJdbcHttpSession.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2019 the original author or authors.
2+
* Copyright 2014-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -98,7 +98,7 @@
9898
* @return the session cleanup cron expression
9999
* @since 2.0.0
100100
*/
101-
String cleanupCron() default JdbcHttpSessionConfiguration.DEFAULT_CLEANUP_CRON;
101+
String cleanupCron() default JdbcIndexedSessionRepository.DEFAULT_CLEANUP_CRON;
102102

103103
/**
104104
* Flush mode for the sessions. The default is {@code ON_SAVE} which only updates the

0 commit comments

Comments
 (0)