Author: fschumacher Date: Sat Nov 21 13:29:18 2015 New Revision: 1715521 URL: http://svn.apache.org/viewvc?rev=1715521&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=34319
It is not only inefficient memory wise to load all sessions in processExpire, it will put more pressure on the lock for JDBCStore, also. Modified: tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java tomcat/trunk/java/org/apache/catalina/session/StoreBase.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java?rev=1715521&r1=1715520&r2=1715521&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java (original) +++ tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java Sat Nov 21 13:29:18 2015 @@ -156,11 +156,6 @@ public class JDBCStore extends StoreBase protected PreparedStatement preparedSizeSql = null; /** - * Variable to hold the <code>keys()</code> prepared statement. - */ - protected PreparedStatement preparedKeysSql = null; - - /** * Variable to hold the <code>save()</code> prepared statement. */ protected PreparedStatement preparedSaveSql = null; @@ -458,17 +453,28 @@ public class JDBCStore extends StoreBase // --------------------------------------------------------- Public Methods + @Override + public String[] expiredKeys() throws IOException { + return keys(true); + } + + @Override + public String[] keys() throws IOException { + return keys(false); + } + /** * Return an array containing the session identifiers of all Sessions * currently saved in this Store. If there are no such Sessions, a * zero-length array is returned. * + * @param expiredOnly flag, whether only keys of expired sessions should + * be returned * @return array containing the list of session IDs * * @exception IOException if an input/output error occurred */ - @Override - public String[] keys() throws IOException { + private String[] keys(boolean expiredOnly) throws IOException { String keys[] = null; synchronized (this) { int numberOfTries = 2; @@ -479,24 +485,29 @@ public class JDBCStore extends StoreBase return new String[0]; } try { - if (preparedKeysSql == null) { - String keysSql = "SELECT " + sessionIdCol + " FROM " - + sessionTable + " WHERE " + sessionAppCol - + " = ?"; - preparedKeysSql = _conn.prepareStatement(keysSql); - } - preparedKeysSql.setString(1, getName()); - try (ResultSet rst = preparedKeysSql.executeQuery()) { - ArrayList<String> tmpkeys = new ArrayList<>(); - if (rst != null) { - while (rst.next()) { - tmpkeys.add(rst.getString(1)); + String keysSql = "SELECT " + sessionIdCol + " FROM " + + sessionTable + " WHERE " + sessionAppCol + " = ?"; + if (expiredOnly) { + keysSql += " AND (" + sessionLastAccessedCol + " + " + + sessionMaxInactiveCol + " * 1000 < ?)"; + } + try (PreparedStatement preparedKeysSql = _conn.prepareStatement(keysSql)) { + preparedKeysSql.setString(1, getName()); + if (expiredOnly) { + preparedKeysSql.setLong(2, System.currentTimeMillis()); + } + try (ResultSet rst = preparedKeysSql.executeQuery()) { + ArrayList<String> tmpkeys = new ArrayList<>(); + if (rst != null) { + while (rst.next()) { + tmpkeys.add(rst.getString(1)); + } } + keys = tmpkeys.toArray(new String[tmpkeys.size()]); + // Break out after the finally block + numberOfTries = 0; } - keys = tmpkeys.toArray(new String[tmpkeys.size()]); - // Break out after the finally block - numberOfTries = 0; } } catch (SQLException e) { manager.getContext().getLogger().error(sm.getString(getStoreName() + ".SQLException", e)); @@ -934,13 +945,6 @@ public class JDBCStore extends StoreBase this.preparedSizeSql = null; try { - preparedKeysSql.close(); - } catch (Throwable f) { - ExceptionUtils.handleThrowable(f); - } - this.preparedKeysSql = null; - - try { preparedSaveSql.close(); } catch (Throwable f) { ExceptionUtils.handleThrowable(f); Modified: tomcat/trunk/java/org/apache/catalina/session/StoreBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/StoreBase.java?rev=1715521&r1=1715520&r2=1715521&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/session/StoreBase.java (original) +++ tomcat/trunk/java/org/apache/catalina/session/StoreBase.java Sat Nov 21 13:29:18 2015 @@ -113,6 +113,18 @@ public abstract class StoreBase extends } /** + * Get only those keys of sessions, that are saved in the Store and are to + * be expired. + * + * @return list of session keys, that are to be expired + * @throws IOException + * if an input-/output error occurred + */ + public String[] expiredKeys() throws IOException { + return keys(); + } + + /** * Called by our background reaper thread to check if Sessions * saved in our store are subject of being expired. If so expire * the Session and remove it from the Store. @@ -126,7 +138,7 @@ public abstract class StoreBase extends } try { - keys = keys(); + keys = expiredKeys(); } catch (IOException e) { manager.getContext().getLogger().error("Error getting keys", e); return; Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1715521&r1=1715520&r2=1715521&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Sat Nov 21 13:29:18 2015 @@ -67,6 +67,11 @@ <code>Engine</code> rather than a <code>Container</code>. (markt) </scode> <fix> + <bug>34319</bug>: Only load those keys in <code>StoreBase.processExpire</code> + from JDBCStore, that are old enough, to be expired. Based on a patch + by Tom Anderson. (fschumacher) + </fix> + <fix> <bug>58629</bug>: Allow an embedded Tomcat instance to start when the <code>Service</code> has no <code>Engine</code> configured. (markt) </fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org