Skip to content

Commit e721efe

Browse files
vpavicvpavic
committed
Optimize insert attribute statement in JdbcIndexedSessionRepository
At present, the SQL statement used to insert a session attribute record contains a nested select statement that verifies the existence of parent record in the session table. Such approach can be susceptible to deadlocks on certain RDMBSs. This commit optimizes the SQL statement used to insert session attribute so that it doesn't perform a nested select statement. Closes: #1550
1 parent 0111c6e commit e721efe

7 files changed

+80
-59
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ public class Db2JdbcIndexedSessionRepositoryCustomizer
3232
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = ""
3333
+ "MERGE INTO %TABLE_NAME%_ATTRIBUTES SA "
3434
+ "USING ( "
35-
+ " SELECT PRIMARY_ID, ?, ? "
36-
+ " FROM %TABLE_NAME% "
37-
+ " WHERE SESSION_ID = ? "
35+
+ " VALUES (?, ?, ?) "
3836
+ ") A (SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) "
3937
+ "ON (SA.SESSION_PRIMARY_ID = A.SESSION_PRIMARY_ID and SA.ATTRIBUTE_NAME = A.ATTRIBUTE_NAME) "
4038
+ "WHEN MATCHED THEN "

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

Lines changed: 65 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import org.springframework.core.serializer.support.DeserializingConverter;
4141
import org.springframework.core.serializer.support.SerializingConverter;
4242
import org.springframework.dao.DataAccessException;
43+
import org.springframework.dao.DataIntegrityViolationException;
44+
import org.springframework.dao.DuplicateKeyException;
4345
import org.springframework.jdbc.core.BatchPreparedStatementSetter;
4446
import org.springframework.jdbc.core.JdbcOperations;
4547
import org.springframework.jdbc.core.ResultSetExtractor;
@@ -139,55 +141,64 @@ public class JdbcIndexedSessionRepository
139141
private static final String SPRING_SECURITY_CONTEXT = "SPRING_SECURITY_CONTEXT";
140142

141143
// @formatter:off
142-
private static final String CREATE_SESSION_QUERY = "INSERT INTO %TABLE_NAME%(PRIMARY_ID, SESSION_ID, CREATION_TIME, LAST_ACCESS_TIME, MAX_INACTIVE_INTERVAL, EXPIRY_TIME, PRINCIPAL_NAME) "
144+
private static final String CREATE_SESSION_QUERY = ""
145+
+ "INSERT INTO %TABLE_NAME% (PRIMARY_ID, SESSION_ID, CREATION_TIME, LAST_ACCESS_TIME, MAX_INACTIVE_INTERVAL, EXPIRY_TIME, PRINCIPAL_NAME) "
143146
+ "VALUES (?, ?, ?, ?, ?, ?, ?)";
144147
// @formatter:on
145148

146149
// @formatter:off
147-
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = "INSERT INTO %TABLE_NAME%_ATTRIBUTES(SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) "
148-
+ "SELECT PRIMARY_ID, ?, ? "
149-
+ "FROM %TABLE_NAME% "
150-
+ "WHERE SESSION_ID = ?";
150+
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = ""
151+
+ "INSERT INTO %TABLE_NAME%_ATTRIBUTES (SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) "
152+
+ "VALUES (?, ?, ?)";
151153
// @formatter:on
152154

153155
// @formatter:off
154-
private static final String GET_SESSION_QUERY = "SELECT S.PRIMARY_ID, S.SESSION_ID, S.CREATION_TIME, S.LAST_ACCESS_TIME, S.MAX_INACTIVE_INTERVAL, SA.ATTRIBUTE_NAME, SA.ATTRIBUTE_BYTES "
156+
private static final String GET_SESSION_QUERY = ""
157+
+ "SELECT S.PRIMARY_ID, S.SESSION_ID, S.CREATION_TIME, S.LAST_ACCESS_TIME, S.MAX_INACTIVE_INTERVAL, SA.ATTRIBUTE_NAME, SA.ATTRIBUTE_BYTES "
155158
+ "FROM %TABLE_NAME% S "
156-
+ "LEFT OUTER JOIN %TABLE_NAME%_ATTRIBUTES SA ON S.PRIMARY_ID = SA.SESSION_PRIMARY_ID "
159+
+ "LEFT JOIN %TABLE_NAME%_ATTRIBUTES SA ON S.PRIMARY_ID = SA.SESSION_PRIMARY_ID "
157160
+ "WHERE S.SESSION_ID = ?";
158161
// @formatter:on
159162

160163
// @formatter:off
161-
private static final String UPDATE_SESSION_QUERY = "UPDATE %TABLE_NAME% SET SESSION_ID = ?, LAST_ACCESS_TIME = ?, MAX_INACTIVE_INTERVAL = ?, EXPIRY_TIME = ?, PRINCIPAL_NAME = ? "
164+
private static final String UPDATE_SESSION_QUERY = ""
165+
+ "UPDATE %TABLE_NAME% "
166+
+ "SET SESSION_ID = ?, LAST_ACCESS_TIME = ?, MAX_INACTIVE_INTERVAL = ?, EXPIRY_TIME = ?, PRINCIPAL_NAME = ? "
162167
+ "WHERE PRIMARY_ID = ?";
163168
// @formatter:on
164169

165170
// @formatter:off
166-
private static final String UPDATE_SESSION_ATTRIBUTE_QUERY = "UPDATE %TABLE_NAME%_ATTRIBUTES SET ATTRIBUTE_BYTES = ? "
171+
private static final String UPDATE_SESSION_ATTRIBUTE_QUERY = ""
172+
+ "UPDATE %TABLE_NAME%_ATTRIBUTES "
173+
+ "SET ATTRIBUTE_BYTES = ? "
167174
+ "WHERE SESSION_PRIMARY_ID = ? "
168175
+ "AND ATTRIBUTE_NAME = ?";
169176
// @formatter:on
170177

171178
// @formatter:off
172-
private static final String DELETE_SESSION_ATTRIBUTE_QUERY = "DELETE FROM %TABLE_NAME%_ATTRIBUTES "
179+
private static final String DELETE_SESSION_ATTRIBUTE_QUERY = ""
180+
+ "DELETE FROM %TABLE_NAME%_ATTRIBUTES "
173181
+ "WHERE SESSION_PRIMARY_ID = ? "
174182
+ "AND ATTRIBUTE_NAME = ?";
175183
// @formatter:on
176184

177185
// @formatter:off
178-
private static final String DELETE_SESSION_QUERY = "DELETE FROM %TABLE_NAME% "
186+
private static final String DELETE_SESSION_QUERY = ""
187+
+ "DELETE FROM %TABLE_NAME% "
179188
+ "WHERE SESSION_ID = ?";
180189
// @formatter:on
181190

182191
// @formatter:off
183-
private static final String LIST_SESSIONS_BY_PRINCIPAL_NAME_QUERY = "SELECT S.PRIMARY_ID, S.SESSION_ID, S.CREATION_TIME, S.LAST_ACCESS_TIME, S.MAX_INACTIVE_INTERVAL, SA.ATTRIBUTE_NAME, SA.ATTRIBUTE_BYTES "
192+
private static final String LIST_SESSIONS_BY_PRINCIPAL_NAME_QUERY = ""
193+
+ "SELECT S.PRIMARY_ID, S.SESSION_ID, S.CREATION_TIME, S.LAST_ACCESS_TIME, S.MAX_INACTIVE_INTERVAL, SA.ATTRIBUTE_NAME, SA.ATTRIBUTE_BYTES "
184194
+ "FROM %TABLE_NAME% S "
185-
+ "LEFT OUTER JOIN %TABLE_NAME%_ATTRIBUTES SA ON S.PRIMARY_ID = SA.SESSION_PRIMARY_ID "
195+
+ "LEFT JOIN %TABLE_NAME%_ATTRIBUTES SA ON S.PRIMARY_ID = SA.SESSION_PRIMARY_ID "
186196
+ "WHERE S.PRINCIPAL_NAME = ?";
187197
// @formatter:on
188198

189199
// @formatter:off
190-
private static final String DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY = "DELETE FROM %TABLE_NAME% "
200+
private static final String DELETE_SESSIONS_BY_EXPIRY_TIME_QUERY = ""
201+
+ "DELETE FROM %TABLE_NAME% "
191202
+ "WHERE EXPIRY_TIME < ?";
192203
// @formatter:on
193204

@@ -463,30 +474,49 @@ private void insertSessionAttributes(JdbcSession session, List<String> attribute
463474
Assert.notEmpty(attributeNames, "attributeNames must not be null or empty");
464475
try (LobCreator lobCreator = this.lobHandler.getLobCreator()) {
465476
if (attributeNames.size() > 1) {
466-
this.jdbcOperations.batchUpdate(this.createSessionAttributeQuery, new BatchPreparedStatementSetter() {
467-
468-
@Override
469-
public void setValues(PreparedStatement ps, int i) throws SQLException {
470-
String attributeName = attributeNames.get(i);
471-
ps.setString(1, attributeName);
472-
lobCreator.setBlobAsBytes(ps, 2, serialize(session.getAttribute(attributeName)));
473-
ps.setString(3, session.getId());
474-
}
475-
476-
@Override
477-
public int getBatchSize() {
478-
return attributeNames.size();
479-
}
477+
try {
478+
this.jdbcOperations.batchUpdate(this.createSessionAttributeQuery,
479+
new BatchPreparedStatementSetter() {
480+
481+
@Override
482+
public void setValues(PreparedStatement ps, int i) throws SQLException {
483+
String attributeName = attributeNames.get(i);
484+
ps.setString(1, session.primaryKey);
485+
ps.setString(2, attributeName);
486+
lobCreator.setBlobAsBytes(ps, 3, serialize(session.getAttribute(attributeName)));
487+
}
488+
489+
@Override
490+
public int getBatchSize() {
491+
return attributeNames.size();
492+
}
480493

481-
});
494+
});
495+
}
496+
catch (DuplicateKeyException ex) {
497+
throw ex;
498+
}
499+
catch (DataIntegrityViolationException ex) {
500+
// parent record not found - we are ignoring this error because we
501+
// assume that a concurrent request has removed the session
502+
}
482503
}
483504
else {
484-
this.jdbcOperations.update(this.createSessionAttributeQuery, (ps) -> {
485-
String attributeName = attributeNames.get(0);
486-
ps.setString(1, attributeName);
487-
lobCreator.setBlobAsBytes(ps, 2, serialize(session.getAttribute(attributeName)));
488-
ps.setString(3, session.getId());
489-
});
505+
try {
506+
this.jdbcOperations.update(this.createSessionAttributeQuery, (ps) -> {
507+
String attributeName = attributeNames.get(0);
508+
ps.setString(1, session.primaryKey);
509+
ps.setString(2, attributeName);
510+
lobCreator.setBlobAsBytes(ps, 3, serialize(session.getAttribute(attributeName)));
511+
});
512+
}
513+
catch (DuplicateKeyException ex) {
514+
throw ex;
515+
}
516+
catch (DataIntegrityViolationException ex) {
517+
// parent record not found - we are ignoring this error because we
518+
// assume that a concurrent request has removed the session
519+
}
490520
}
491521
}
492522
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ public class MySqlJdbcIndexedSessionRepositoryCustomizer
3131
// @formatter:off
3232
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = ""
3333
+ "INSERT INTO %TABLE_NAME%_ATTRIBUTES (SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) "
34-
+ " SELECT PRIMARY_ID, ?, ? "
35-
+ " FROM %TABLE_NAME% "
36-
+ " WHERE SESSION_ID = ? "
34+
+ "VALUES (?, ?, ?) "
3735
+ "ON DUPLICATE KEY UPDATE ATTRIBUTE_BYTES = VALUES(ATTRIBUTE_BYTES)";
3836
// @formatter:on
3937

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ public class OracleJdbcIndexedSessionRepositoryCustomizer
3232
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = ""
3333
+ "MERGE INTO %TABLE_NAME%_ATTRIBUTES SA "
3434
+ "USING ( "
35-
+ " SELECT PRIMARY_ID AS SESSION_PRIMARY_ID, ? AS ATTRIBUTE_NAME, ? AS ATTRIBUTE_BYTES "
36-
+ " FROM %TABLE_NAME% "
37-
+ " WHERE SESSION_ID = ? "
35+
+ " SELECT ? AS SESSION_PRIMARY_ID, ? AS ATTRIBUTE_NAME, ? AS ATTRIBUTE_BYTES "
36+
+ " FROM DUAL "
3837
+ ") A "
3938
+ "ON (SA.SESSION_PRIMARY_ID = A.SESSION_PRIMARY_ID and SA.ATTRIBUTE_NAME = A.ATTRIBUTE_NAME) "
4039
+ "WHEN MATCHED THEN "

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ public class PostgreSqlJdbcIndexedSessionRepositoryCustomizer
3131
// @formatter:off
3232
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = ""
3333
+ "INSERT INTO %TABLE_NAME%_ATTRIBUTES (SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) "
34-
+ " SELECT PRIMARY_ID, ?, ? "
35-
+ " FROM %TABLE_NAME% "
36-
+ " WHERE SESSION_ID = ? "
34+
+ "VALUES (?, ?, ?) "
3735
+ "ON CONFLICT (SESSION_PRIMARY_ID, ATTRIBUTE_NAME) "
3836
+ "DO UPDATE SET ATTRIBUTE_BYTES = EXCLUDED.ATTRIBUTE_BYTES";
3937
// @formatter:on

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ public class SqlServerJdbcIndexedSessionRepositoryCustomizer
3232
private static final String CREATE_SESSION_ATTRIBUTE_QUERY = ""
3333
+ "MERGE INTO %TABLE_NAME%_ATTRIBUTES SA "
3434
+ "USING ( "
35-
+ " SELECT PRIMARY_ID, ?, ? "
36-
+ " FROM %TABLE_NAME% "
37-
+ " WHERE SESSION_ID = ? "
35+
+ " VALUES (?, ?, ?) "
3836
+ ") A (SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) "
3937
+ "ON (SA.SESSION_PRIMARY_ID = A.SESSION_PRIMARY_ID and SA.ATTRIBUTE_NAME = A.ATTRIBUTE_NAME) "
4038
+ "WHEN MATCHED THEN "

‎spring-session-jdbc/src/test/java/org/springframework/session/jdbc/JdbcIndexedSessionRepositoryTests.java

Lines changed: 9 additions & 9 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-2020 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.
@@ -290,9 +290,9 @@ void saveNewWithSingleAttribute() {
290290
this.repository.save(session);
291291

292292
assertThat(session.isNew()).isFalse();
293-
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION("),
293+
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION ("),
294294
isA(PreparedStatementSetter.class));
295-
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
295+
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES ("),
296296
isA(PreparedStatementSetter.class));
297297
verifyNoMoreInteractions(this.jdbcOperations);
298298
}
@@ -306,9 +306,9 @@ void saveNewWithMultipleAttributes() {
306306
this.repository.save(session);
307307

308308
assertThat(session.isNew()).isFalse();
309-
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION("),
309+
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION ("),
310310
isA(PreparedStatementSetter.class));
311-
verify(this.jdbcOperations, times(1)).batchUpdate(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
311+
verify(this.jdbcOperations, times(1)).batchUpdate(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES ("),
312312
isA(BatchPreparedStatementSetter.class));
313313
verifyNoMoreInteractions(this.jdbcOperations);
314314
}
@@ -321,7 +321,7 @@ void saveUpdatedAddSingleAttribute() {
321321
this.repository.save(session);
322322

323323
assertThat(session.isNew()).isFalse();
324-
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
324+
verify(this.jdbcOperations, times(1)).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES ("),
325325
isA(PreparedStatementSetter.class));
326326
verifyNoMoreInteractions(this.jdbcOperations);
327327
}
@@ -335,7 +335,7 @@ void saveUpdatedAddMultipleAttributes() {
335335
this.repository.save(session);
336336

337337
assertThat(session.isNew()).isFalse();
338-
verify(this.jdbcOperations, times(1)).batchUpdate(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
338+
verify(this.jdbcOperations, times(1)).batchUpdate(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES ("),
339339
isA(BatchPreparedStatementSetter.class));
340340
verifyNoMoreInteractions(this.jdbcOperations);
341341
}
@@ -424,7 +424,7 @@ void saveUpdatedAddAndModifyAttribute() {
424424
this.repository.save(session);
425425

426426
assertThat(session.isNew()).isFalse();
427-
verify(this.jdbcOperations).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
427+
verify(this.jdbcOperations).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES ("),
428428
isA(PreparedStatementSetter.class));
429429
verifyNoMoreInteractions(this.jdbcOperations);
430430
}
@@ -679,7 +679,7 @@ void flushModeImmediateSetAttribute() {
679679
JdbcSession session = this.repository.new JdbcSession(new MapSession(), "primaryKey", false);
680680
String attrName = "someAttribute";
681681
session.setAttribute(attrName, "someValue");
682-
verify(this.jdbcOperations).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES("),
682+
verify(this.jdbcOperations).update(startsWith("INSERT INTO SPRING_SESSION_ATTRIBUTES ("),
683683
isA(PreparedStatementSetter.class));
684684
verifyNoMoreInteractions(this.jdbcOperations);
685685
}

0 commit comments

Comments
 (0)