Skip to content

Commit 954a40f

Browse files
vpavicrwinch
vpavic
authored and
rwinch
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 fb66cf3 commit 954a40f

File tree

10 files changed

+162
-71
lines changed

10 files changed

+162
-71
lines changed

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

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.apache.commons.logging.Log;
2828
import org.apache.commons.logging.LogFactory;
2929

30+
import org.springframework.beans.factory.DisposableBean;
31+
import org.springframework.beans.factory.InitializingBean;
3032
import org.springframework.context.ApplicationEvent;
3133
import org.springframework.context.ApplicationEventPublisher;
3234
import org.springframework.core.NestedExceptionUtils;
@@ -38,6 +40,10 @@
3840
import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer;
3941
import org.springframework.data.redis.serializer.RedisSerializer;
4042
import org.springframework.data.redis.util.ByteUtils;
43+
import org.springframework.scheduling.annotation.Scheduled;
44+
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
45+
import org.springframework.scheduling.support.CronExpression;
46+
import org.springframework.scheduling.support.CronTrigger;
4147
import org.springframework.session.DelegatingIndexResolver;
4248
import org.springframework.session.FindByIndexNameSessionRepository;
4349
import org.springframework.session.FlushMode;
@@ -249,12 +255,18 @@
249255
* @since 2.2.0
250256
*/
251257
public class RedisIndexedSessionRepository
252-
implements FindByIndexNameSessionRepository<RedisIndexedSessionRepository.RedisSession>, MessageListener {
258+
implements FindByIndexNameSessionRepository<RedisIndexedSessionRepository.RedisSession>, MessageListener,
259+
InitializingBean, DisposableBean {
253260

254261
private static final Log logger = LogFactory.getLog(RedisIndexedSessionRepository.class);
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+
@Override
342+
public void afterPropertiesSet() {
343+
if (!Scheduled.CRON_DISABLED.equals(this.cleanupCron)) {
344+
this.taskScheduler = createTaskScheduler();
345+
this.taskScheduler.initialize();
346+
this.taskScheduler.schedule(this::cleanUpExpiredSessions, new CronTrigger(this.cleanupCron));
347+
}
348+
}
349+
350+
private static ThreadPoolTaskScheduler createTaskScheduler() {
351+
ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler();
352+
taskScheduler.setThreadNamePrefix("spring-session-");
353+
return taskScheduler;
354+
}
355+
356+
@Override
357+
public 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
@@ -106,6 +106,6 @@
106106
* The cron expression for expired session cleanup job. By default runs every minute.
107107
* @return the session cleanup cron expression
108108
*/
109-
String cleanupCron() default RedisIndexedHttpSessionConfiguration.DEFAULT_CLEANUP_CRON;
109+
String cleanupCron() default RedisIndexedSessionRepository.DEFAULT_CLEANUP_CRON;
110110

111111
}

‎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.afterPropertiesSet();
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/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
import org.apache.commons.logging.Log;
3535
import org.apache.commons.logging.LogFactory;
3636

37+
import org.springframework.beans.factory.DisposableBean;
38+
import org.springframework.beans.factory.InitializingBean;
3739
import org.springframework.core.convert.ConversionService;
3840
import org.springframework.core.convert.TypeDescriptor;
3941
import org.springframework.core.convert.support.GenericConversionService;
@@ -48,6 +50,10 @@
4850
import org.springframework.jdbc.support.lob.DefaultLobHandler;
4951
import org.springframework.jdbc.support.lob.LobCreator;
5052
import org.springframework.jdbc.support.lob.LobHandler;
53+
import org.springframework.scheduling.annotation.Scheduled;
54+
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
55+
import org.springframework.scheduling.support.CronExpression;
56+
import org.springframework.scheduling.support.CronTrigger;
5157
import org.springframework.session.DelegatingIndexResolver;
5258
import org.springframework.session.FindByIndexNameSessionRepository;
5359
import org.springframework.session.FlushMode;
@@ -130,14 +136,19 @@
130136
* @author Craig Andrews
131137
* @since 2.2.0
132138
*/
133-
public class JdbcIndexedSessionRepository
134-
implements FindByIndexNameSessionRepository<JdbcIndexedSessionRepository.JdbcSession> {
139+
public class JdbcIndexedSessionRepository implements
140+
FindByIndexNameSessionRepository<JdbcIndexedSessionRepository.JdbcSession>, InitializingBean, DisposableBean {
135141

136142
/**
137143
* The default name of database table used by Spring Session to store sessions.
138144
*/
139145
public static final String DEFAULT_TABLE_NAME = "SPRING_SESSION";
140146

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

143154
private static final String CREATE_SESSION_QUERY = """
@@ -241,6 +252,10 @@ public class JdbcIndexedSessionRepository
241252

242253
private SaveMode saveMode = SaveMode.ON_SET_ATTRIBUTE;
243254

255+
private String cleanupCron = DEFAULT_CLEANUP_CRON;
256+
257+
private ThreadPoolTaskScheduler taskScheduler;
258+
244259
/**
245260
* Create a new {@link JdbcIndexedSessionRepository} instance which uses the provided
246261
* {@link JdbcOperations} and {@link TransactionOperations} to manage sessions.
@@ -255,6 +270,28 @@ public JdbcIndexedSessionRepository(JdbcOperations jdbcOperations, TransactionOp
255270
prepareQueries();
256271
}
257272

273+
@Override
274+
public void afterPropertiesSet() {
275+
if (!Scheduled.CRON_DISABLED.equals(this.cleanupCron)) {
276+
this.taskScheduler = createTaskScheduler();
277+
this.taskScheduler.initialize();
278+
this.taskScheduler.schedule(this::cleanUpExpiredSessions, new CronTrigger(this.cleanupCron));
279+
}
280+
}
281+
282+
private static ThreadPoolTaskScheduler createTaskScheduler() {
283+
ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler();
284+
taskScheduler.setThreadNamePrefix("spring-session-");
285+
return taskScheduler;
286+
}
287+
288+
@Override
289+
public void destroy() {
290+
if (this.taskScheduler != null) {
291+
this.taskScheduler.destroy();
292+
}
293+
}
294+
258295
/**
259296
* Set the name of database table used to store sessions.
260297
* @param tableName the database table name
@@ -397,6 +434,21 @@ public void setSaveMode(SaveMode saveMode) {
397434
this.saveMode = saveMode;
398435
}
399436

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
* @return the session cleanup cron expression
9797
* @since 2.0.0
9898
*/
99-
String cleanupCron() default JdbcHttpSessionConfiguration.DEFAULT_CLEANUP_CRON;
99+
String cleanupCron() default JdbcIndexedSessionRepository.DEFAULT_CLEANUP_CRON;
100100

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

0 commit comments

Comments
 (0)