From 2c830a623cdfbdbf586e7a4d19690dc9048216c3 Mon Sep 17 00:00:00 2001 From: Christopher Schultz Date: Wed, 8 Mar 2023 10:33:02 -0500 Subject: [PATCH 1/5] Use SELECT...FOR UPDATE to update session data in the database instead of DELETE, INSERT. --- .../catalina/session/DataSourceStore.java | 102 ++++++++++++++---- 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/java/org/apache/catalina/session/DataSourceStore.java b/java/org/apache/catalina/session/DataSourceStore.java index c46aa2b17c68..bd05962a9122 100644 --- a/java/org/apache/catalina/session/DataSourceStore.java +++ b/java/org/apache/catalina/session/DataSourceStore.java @@ -597,12 +597,17 @@ public void clear() throws IOException { @Override public void save(Session session) throws IOException { ByteArrayOutputStream bos = null; - String saveSql = "INSERT INTO " + sessionTable + " (" - + sessionIdCol + ", " + sessionAppCol + ", " - + sessionDataCol + ", " + sessionValidCol - + ", " + sessionMaxInactiveCol + ", " - + sessionLastAccessedCol - + ") VALUES (?, ?, ?, ?, ?, ?)"; + String saveSql = "SELECT " + sessionIdCol + + ", " + sessionAppCol + + ", " + sessionIdCol + + ", " + sessionDataCol + + ", " + sessionValidCol + + ", " + sessionMaxInactiveCol + + ", " + sessionLastAccessedCol + + " FROM " + sessionTable + + " WHERE " + sessionAppCol + "=?" + + " AND " + sessionIdCol + "=? FOR UPDATE" + ; synchronized (session) { int numberOfTries = 2; @@ -613,11 +618,6 @@ public void save(Session session) throws IOException { } try { - // If sessions already exist in DB, remove and insert again. - // TODO: - // * Check if ID exists in database and if so use UPDATE. - remove(session.getIdInternal(), _conn); - bos = new ByteArrayOutputStream(); try (ObjectOutputStream oos = new ObjectOutputStream(new BufferedOutputStream(bos))) { @@ -626,15 +626,77 @@ public void save(Session session) throws IOException { byte[] obs = bos.toByteArray(); int size = obs.length; try (ByteArrayInputStream bis = new ByteArrayInputStream(obs, 0, size); - InputStream in = new BufferedInputStream(bis, size); - PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql)) { - preparedSaveSql.setString(1, session.getIdInternal()); - preparedSaveSql.setString(2, getName()); - preparedSaveSql.setBinaryStream(3, in, size); - preparedSaveSql.setString(4, session.isValid() ? "1" : "0"); - preparedSaveSql.setInt(5, session.getMaxInactiveInterval()); - preparedSaveSql.setLong(6, session.getLastAccessedTime()); - preparedSaveSql.execute(); + InputStream in = new BufferedInputStream(bis, size); + PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE)) { + + // Store auto-commit state + boolean autoCommit = _conn.getAutoCommit(); + + try { + if(autoCommit) { + _conn.setAutoCommit(false); // BEGIN TRANSACTION + } + + preparedSaveSql.setString(1, getName()); + preparedSaveSql.setString(2, session.getIdInternal()); + + ResultSet rs = preparedSaveSql.executeQuery(); + + if(rs.next()) { + // Session already exists in the db; update the various fields + rs.updateBinaryStream(sessionDataCol, in, size); + rs.updateString(sessionValidCol, session.isValid() ? "1" : "0"); + rs.updateInt(sessionMaxInactiveCol, session.getMaxInactiveInterval()); + rs.updateLong(sessionLastAccessedCol, session.getLastAccessedTime()); + + rs.updateRow(); + } else { + // Session does not exist. Insert. + rs.moveToInsertRow(); + + rs.updateString(sessionAppCol, getName()); + rs.updateString(sessionIdCol, session.getIdInternal()); + rs.updateBinaryStream(sessionIdCol, in, size); + rs.updateString(sessionValidCol, session.isValid() ? "1" : "0"); + rs.updateInt(sessionMaxInactiveCol, session.getMaxInactiveInterval()); + rs.updateLong(sessionLastAccessedCol, session.getLastAccessedTime()); + + rs.updateRow(); + } + + _conn.commit(); + } catch (SQLException sqle) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", sqle)); + try { + _conn.rollback(); + } catch (SQLException sqle1) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", sqle1)); + } + } catch (RuntimeException rte) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", rte)); + try { + _conn.rollback(); + } catch (SQLException sqle1) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", sqle1)); + } + + throw rte; + } catch (Error e) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", e)); + try { + _conn.rollback(); + } catch (SQLException sqle1) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", sqle1)); + } + + throw e; + } finally { + if(autoCommit) { + // Restore connection auto-commit state + _conn.setAutoCommit(autoCommit); + } + } + // Break out after the finally block numberOfTries = 0; } From d5a7ce41e88a2dc945c19e02c78482ff323176b0 Mon Sep 17 00:00:00 2001 From: Christopher Schultz Date: Thu, 9 Mar 2023 08:26:02 -0500 Subject: [PATCH 2/5] Use ResultSet.insert when inserting. Close ResultSet when done. --- java/org/apache/catalina/session/DataSourceStore.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/java/org/apache/catalina/session/DataSourceStore.java b/java/org/apache/catalina/session/DataSourceStore.java index bd05962a9122..4a1e1685eb8f 100644 --- a/java/org/apache/catalina/session/DataSourceStore.java +++ b/java/org/apache/catalina/session/DataSourceStore.java @@ -627,7 +627,8 @@ public void save(Session session) throws IOException { int size = obs.length; try (ByteArrayInputStream bis = new ByteArrayInputStream(obs, 0, size); InputStream in = new BufferedInputStream(bis, size); - PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE)) { + PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE); + ResultSet rs = null) { // Store auto-commit state boolean autoCommit = _conn.getAutoCommit(); @@ -640,7 +641,7 @@ public void save(Session session) throws IOException { preparedSaveSql.setString(1, getName()); preparedSaveSql.setString(2, session.getIdInternal()); - ResultSet rs = preparedSaveSql.executeQuery(); + rs = preparedSaveSql.executeQuery(); if(rs.next()) { // Session already exists in the db; update the various fields @@ -661,7 +662,7 @@ public void save(Session session) throws IOException { rs.updateInt(sessionMaxInactiveCol, session.getMaxInactiveInterval()); rs.updateLong(sessionLastAccessedCol, session.getLastAccessedTime()); - rs.updateRow(); + rs.insertRow(); } _conn.commit(); From 4a129847f577fc025a05692d880ab7e81efeef80 Mon Sep 17 00:00:00 2001 From: Christopher Schultz Date: Thu, 9 Mar 2023 08:33:06 -0500 Subject: [PATCH 3/5] Resources in try-with-resources are final. --- java/org/apache/catalina/session/DataSourceStore.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/session/DataSourceStore.java b/java/org/apache/catalina/session/DataSourceStore.java index 4a1e1685eb8f..cf3401d73d02 100644 --- a/java/org/apache/catalina/session/DataSourceStore.java +++ b/java/org/apache/catalina/session/DataSourceStore.java @@ -627,12 +627,12 @@ public void save(Session session) throws IOException { int size = obs.length; try (ByteArrayInputStream bis = new ByteArrayInputStream(obs, 0, size); InputStream in = new BufferedInputStream(bis, size); - PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE); - ResultSet rs = null) { + PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE)) { // Store auto-commit state boolean autoCommit = _conn.getAutoCommit(); + ResultSet rs = null; try { if(autoCommit) { _conn.setAutoCommit(false); // BEGIN TRANSACTION @@ -692,6 +692,13 @@ public void save(Session session) throws IOException { throw e; } finally { + if(null != rs) { + try { + rs.close(); + } catch (SQLException sqle) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", sqle)); + } + } if(autoCommit) { // Restore connection auto-commit state _conn.setAutoCommit(autoCommit); From bc008453b476441d9c2db6b1e7ac8b5809c8757a Mon Sep 17 00:00:00 2001 From: Christopher Schultz Date: Thu, 9 Mar 2023 08:50:35 -0500 Subject: [PATCH 4/5] Allow choice of session-saving strategy via configuration. --- .../catalina/session/DataSourceStore.java | 129 ++++++++++++++++++ webapps/docs/config/manager.xml | 7 + 2 files changed, 136 insertions(+) diff --git a/java/org/apache/catalina/session/DataSourceStore.java b/java/org/apache/catalina/session/DataSourceStore.java index cf3401d73d02..60425d7e08d1 100644 --- a/java/org/apache/catalina/session/DataSourceStore.java +++ b/java/org/apache/catalina/session/DataSourceStore.java @@ -50,6 +50,22 @@ */ public class DataSourceStore extends StoreBase { + /** + * The DELETE + INSERT saving strategy. + * + * @see #setSaveStrategy + * @see #getSaveStrategy + */ + public static final String SAVE_STRATEGY_DELETE_INSERT = "deleteInsert"; + + /** + * The SELECT...FOR UPDATE saving strategy. + * + * @see #setSaveStrategy + * @see #getSaveStrategy + */ + public static final String SAVE_STRATEGY_SELECT_FOR_UPDATE = "selectForUpdate"; + /** * Context name associated with this Store */ @@ -75,6 +91,10 @@ public class DataSourceStore extends StoreBase { */ protected DataSource dataSource = null; + /** + * Which saving strategy to use: deleteInsert, selectForUpdate, etc. + */ + private String saveStrategy = SAVE_STRATEGY_DELETE_INSERT; // ------------------------------------------------------------ Table & cols @@ -326,6 +346,31 @@ public void setLocalDataSource(boolean localDataSource) { this.localDataSource = localDataSource; } + /** + * Sets the session-saving strategy to use. + * + * E.g. {@link #SAVE_STRATEGY_DELETE_INSERT} or {@link #SAVE_STRATEGY_SELECT_FOR_UPDATE}. + * + * The default is {@link #SAVE_STRATEGY_DELETE_INSERT} for compatibility with all RDMBSs. + * + * @param saveStrategy The saving strategy to use. + */ + public void setSaveStrategy(String saveStrategy) { + this.saveStrategy = saveStrategy; + } + + /** + * Gets the session-saving strategy to use. + * + * E.g. {@link #SAVE_STRATEGY_DELETE_INSERT} or {@link #SAVE_STRATEGY_SELECT_FOR_UPDATE}. + * + * The default is {@link #SAVE_STRATEGY_DELETE_INSERT} for compatibility with all RDMBSs. + * + * @return The saving strategy to use. + */ + public String getSaveStrategy() { + return saveStrategy; + } // --------------------------------------------------------- Public Methods @@ -596,6 +641,90 @@ public void clear() throws IOException { */ @Override public void save(Session session) throws IOException { + if(SAVE_STRATEGY_SELECT_FOR_UPDATE.equals(getSaveStrategy())) { + saveSelectForUpdate(session); + } else { + saveDeleteAndInsert(session); + } + } + + /** + * Saves a session by DELETEing any existing session and INSERTing a new one. + * + * @param session The session to be stored. + * @throws IOException If an input/output error occurs. + */ + private void saveDeleteAndInsert(Session session) throws IOException { + ByteArrayOutputStream bos = null; + String saveSql = "INSERT INTO " + sessionTable + " (" + + sessionIdCol + ", " + sessionAppCol + ", " + + sessionDataCol + ", " + sessionValidCol + + ", " + sessionMaxInactiveCol + ", " + + sessionLastAccessedCol + + ") VALUES (?, ?, ?, ?, ?, ?)"; + + synchronized (session) { + int numberOfTries = 2; + while (numberOfTries > 0) { + Connection _conn = getConnection(); + if (_conn == null) { + return; + } + + try { + // If sessions already exist in DB, remove and insert again. + // TODO: + // * Check if ID exists in database and if so use UPDATE. + remove(session.getIdInternal(), _conn); + + bos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = + new ObjectOutputStream(new BufferedOutputStream(bos))) { + ((StandardSession) session).writeObjectData(oos); + } + byte[] obs = bos.toByteArray(); + int size = obs.length; + try (ByteArrayInputStream bis = new ByteArrayInputStream(obs, 0, size); + InputStream in = new BufferedInputStream(bis, size); + PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql)) { + preparedSaveSql.setString(1, session.getIdInternal()); + preparedSaveSql.setString(2, getName()); + preparedSaveSql.setBinaryStream(3, in, size); + preparedSaveSql.setString(4, session.isValid() ? "1" : "0"); + preparedSaveSql.setInt(5, session.getMaxInactiveInterval()); + preparedSaveSql.setLong(6, session.getLastAccessedTime()); + preparedSaveSql.execute(); + // Break out after the finally block + numberOfTries = 0; + } + } catch (SQLException e) { + manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", e)); + } catch (IOException e) { + // Ignore + } finally { + release(_conn); + } + numberOfTries--; + } + } + + if (manager.getContext().getLogger().isDebugEnabled()) { + manager.getContext().getLogger().debug(sm.getString(getStoreName() + ".saving", + session.getIdInternal(), sessionTable)); + } + } + + /** + * Saves a session using SELECT ... FOR UPDATE to update any existing session + * record or insert a new one. + * + * This should be more efficient with database resources if it is supported + * by the underlying database. + * + * @param session The session to be stored. + * @throws IOException If an input/output error occurs. + */ + private void saveSelectForUpdate(Session session) throws IOException { ByteArrayOutputStream bos = null; String saveSql = "SELECT " + sessionIdCol + ", " + sessionAppCol diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml index 1b7e0b9169a6..6acf1c4fe0b6 100644 --- a/webapps/docs/config/manager.xml +++ b/webapps/docs/config/manager.xml @@ -468,6 +468,13 @@ false: use a global DataSource.

+ +

The session-saving strategy to use, either deleteInsert + or selectForUpdate. The default is deleteInsert + which is compatible with all known RDBMS systems. The selectForUpdate + strategy uses SELECT...FOR UPDATE which may be more efficient + if supported by your RDBMS system.

+

Name of the database column, contained in the specified session table, that contains the Engine, Host, and Web Application Context name in the From b5830710beaf98d15baca515982c470356e79856 Mon Sep 17 00:00:00 2001 From: Christopher Schultz Date: Thu, 9 Mar 2023 09:01:57 -0500 Subject: [PATCH 5/5] Only serialize the session once, even if multiple retries are necessary. --- .../catalina/session/DataSourceStore.java | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/java/org/apache/catalina/session/DataSourceStore.java b/java/org/apache/catalina/session/DataSourceStore.java index 60425d7e08d1..dcf341640e13 100644 --- a/java/org/apache/catalina/session/DataSourceStore.java +++ b/java/org/apache/catalina/session/DataSourceStore.java @@ -641,10 +641,21 @@ public void clear() throws IOException { */ @Override public void save(Session session) throws IOException { + + byte[] sessionBytes; + try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); + ObjectOutputStream oos = + new ObjectOutputStream(new BufferedOutputStream(bos))) { + + ((StandardSession) session).writeObjectData(oos); + + sessionBytes = bos.toByteArray(); + } + if(SAVE_STRATEGY_SELECT_FOR_UPDATE.equals(getSaveStrategy())) { - saveSelectForUpdate(session); + saveSelectForUpdate(session, sessionBytes); } else { - saveDeleteAndInsert(session); + saveDeleteAndInsert(session, sessionBytes); } } @@ -654,8 +665,7 @@ public void save(Session session) throws IOException { * @param session The session to be stored. * @throws IOException If an input/output error occurs. */ - private void saveDeleteAndInsert(Session session) throws IOException { - ByteArrayOutputStream bos = null; + private void saveDeleteAndInsert(Session session, byte[] sessionBytes) throws IOException { String saveSql = "INSERT INTO " + sessionTable + " (" + sessionIdCol + ", " + sessionAppCol + ", " + sessionDataCol + ", " + sessionValidCol @@ -663,6 +673,7 @@ private void saveDeleteAndInsert(Session session) throws IOException { + sessionLastAccessedCol + ") VALUES (?, ?, ?, ?, ?, ?)"; + int sessionBytesLength = sessionBytes.length; synchronized (session) { int numberOfTries = 2; while (numberOfTries > 0) { @@ -677,19 +688,12 @@ private void saveDeleteAndInsert(Session session) throws IOException { // * Check if ID exists in database and if so use UPDATE. remove(session.getIdInternal(), _conn); - bos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = - new ObjectOutputStream(new BufferedOutputStream(bos))) { - ((StandardSession) session).writeObjectData(oos); - } - byte[] obs = bos.toByteArray(); - int size = obs.length; - try (ByteArrayInputStream bis = new ByteArrayInputStream(obs, 0, size); - InputStream in = new BufferedInputStream(bis, size); - PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql)) { + try (ByteArrayInputStream bis = new ByteArrayInputStream(sessionBytes, 0, sessionBytesLength); + InputStream in = new BufferedInputStream(bis, sessionBytesLength); + PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql)) { preparedSaveSql.setString(1, session.getIdInternal()); preparedSaveSql.setString(2, getName()); - preparedSaveSql.setBinaryStream(3, in, size); + preparedSaveSql.setBinaryStream(3, in, sessionBytesLength); preparedSaveSql.setString(4, session.isValid() ? "1" : "0"); preparedSaveSql.setInt(5, session.getMaxInactiveInterval()); preparedSaveSql.setLong(6, session.getLastAccessedTime()); @@ -724,8 +728,7 @@ private void saveDeleteAndInsert(Session session) throws IOException { * @param session The session to be stored. * @throws IOException If an input/output error occurs. */ - private void saveSelectForUpdate(Session session) throws IOException { - ByteArrayOutputStream bos = null; + private void saveSelectForUpdate(Session session, byte[] sessionBytes) throws IOException { String saveSql = "SELECT " + sessionIdCol + ", " + sessionAppCol + ", " + sessionIdCol @@ -738,6 +741,7 @@ private void saveSelectForUpdate(Session session) throws IOException { + " AND " + sessionIdCol + "=? FOR UPDATE" ; + int sessionBytesLength = sessionBytes.length; synchronized (session) { int numberOfTries = 2; while (numberOfTries > 0) { @@ -747,15 +751,8 @@ private void saveSelectForUpdate(Session session) throws IOException { } try { - bos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = - new ObjectOutputStream(new BufferedOutputStream(bos))) { - ((StandardSession) session).writeObjectData(oos); - } - byte[] obs = bos.toByteArray(); - int size = obs.length; - try (ByteArrayInputStream bis = new ByteArrayInputStream(obs, 0, size); - InputStream in = new BufferedInputStream(bis, size); + try (ByteArrayInputStream bis = new ByteArrayInputStream(sessionBytes, 0, sessionBytesLength); + InputStream in = new BufferedInputStream(bis, sessionBytesLength); PreparedStatement preparedSaveSql = _conn.prepareStatement(saveSql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE)) { // Store auto-commit state @@ -774,7 +771,7 @@ private void saveSelectForUpdate(Session session) throws IOException { if(rs.next()) { // Session already exists in the db; update the various fields - rs.updateBinaryStream(sessionDataCol, in, size); + rs.updateBinaryStream(sessionDataCol, in, sessionBytesLength); rs.updateString(sessionValidCol, session.isValid() ? "1" : "0"); rs.updateInt(sessionMaxInactiveCol, session.getMaxInactiveInterval()); rs.updateLong(sessionLastAccessedCol, session.getLastAccessedTime()); @@ -786,7 +783,7 @@ private void saveSelectForUpdate(Session session) throws IOException { rs.updateString(sessionAppCol, getName()); rs.updateString(sessionIdCol, session.getIdInternal()); - rs.updateBinaryStream(sessionIdCol, in, size); + rs.updateBinaryStream(sessionIdCol, in, sessionBytesLength); rs.updateString(sessionValidCol, session.isValid() ? "1" : "0"); rs.updateInt(sessionMaxInactiveCol, session.getMaxInactiveInterval()); rs.updateLong(sessionLastAccessedCol, session.getLastAccessedTime());