Author: markt Date: Wed Apr 6 20:07:05 2016 New Revision: 1738039 URL: http://svn.apache.org/viewvc?rev=1738039&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59269 Correct the implementation of PersistentManagerBase so that minIdleSwap functions as designed and sessions are swapped out to keep the active session count below maxActiveSessions
Added: tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java (with props) Modified: tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java tomcat/trunk/webapps/docs/changelog.xml tomcat/trunk/webapps/docs/config/manager.xml Modified: tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=1738039&r1=1738038&r2=1738039&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java (original) +++ tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java Wed Apr 6 20:07:05 2016 @@ -162,16 +162,18 @@ public abstract class PersistentManagerB /** - * Minimum time a session must be idle before it is swapped to disk. - * This overrides maxActiveSessions, to prevent thrashing if there are lots - * of active sessions. Setting to {@code -1} means it's ignored. + * The minimum time in seconds a session must be idle before it is eligible + * to be swapped to disk to keep the active session count below + * maxActiveSessions. Setting to {@code -1} means sessions will not be + * swapped out to keep the active session count down. */ protected int minIdleSwap = -1; + /** - * The maximum time a session may be idle before it should be swapped - * to file just on general principle. Setting this to {@code -1} means sessions - * should not be forced out. + * The maximum time in seconds a session may be idle before it is eligible + * to be swapped to disk due to inactivity. Setting this to {@code -1} means + * sessions should not be swapped out just because of inactivity. */ protected int maxIdleSwap = -1; @@ -233,19 +235,20 @@ public abstract class PersistentManagerB /** - * @return The time in seconds after which a session should be swapped out of - * memory to disk. + * @return The maximum time in seconds a session may be idle before it is + * eligible to be swapped to disk due to inactivity. A value of {@code -1} + * means sessions should not be swapped out just because of inactivity. */ public int getMaxIdleSwap() { - return maxIdleSwap; - } /** - * Sets the time in seconds after which a session should be swapped out of - * memory to disk. + * Sets the maximum time in seconds a session may be idle before it is + * eligible to be swapped to disk due to inactivity. Setting this to + * {@code -1} means sessions should not be swapped out just because of + * inactivity. * * @param max time in seconds to wait for possible swap out */ @@ -258,26 +261,25 @@ public abstract class PersistentManagerB support.firePropertyChange("maxIdleSwap", Integer.valueOf(oldMaxIdleSwap), Integer.valueOf(this.maxIdleSwap)); - } /** - * @return The minimum time in seconds that a session must be idle before - * it can be swapped out of memory, or {@code -1} if it can be swapped out - * at any time. + * @return The minimum time in seconds a session must be idle before it is + * eligible to be swapped to disk to keep the active session count below + * maxActiveSessions. A value of {@code -1} means sessions will not be + * swapped out to keep the active session count down. */ public int getMinIdleSwap() { - return minIdleSwap; - } /** - * Sets the minimum time in seconds that a session must be idle before - * it can be swapped out of memory due to maxActiveSession. Set it to {@code -1} - * if it can be swapped out at any time. + * Sets the minimum time in seconds a session must be idle before it is + * eligible to be swapped to disk to keep the active session count below + * maxActiveSessions. Setting to {@code -1} means sessions will not be + * swapped out to keep the active session count down. * * @param min time in seconds before a possible swap out */ @@ -941,7 +943,9 @@ public abstract class PersistentManagerB Session sessions[] = findSessions(); // FIXME: Smarter algorithm (LRU) - if (getMaxActiveSessions() >= sessions.length) + int limit = (int) (getMaxActiveSessions() * 0.9); + + if (limit >= sessions.length) return; if(log.isDebugEnabled()) @@ -949,7 +953,7 @@ public abstract class PersistentManagerB ("persistentManager.tooManyActive", Integer.valueOf(sessions.length))); - int toswap = sessions.length - getMaxActiveSessions(); + int toswap = sessions.length - limit; for (int i = 0; i < sessions.length && toswap > 0; i++) { StandardSession session = (StandardSession) sessions[i]; Added: tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java?rev=1738039&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java (added) +++ tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java Wed Apr 6 20:07:05 2016 @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.session; + +import org.apache.catalina.Context; +import org.apache.catalina.Host; +import org.apache.tomcat.unittest.TesterContext; +import org.apache.tomcat.unittest.TesterHost; +import org.junit.Assert; +import org.junit.Test; + +public class TestPersistentManager { + + @Test + public void testMinIdleSwap() throws Exception { + PersistentManager manager = new PersistentManager(); + manager.setStore(new TesterStore()); + + Host host = new TesterHost(); + Context context = new TesterContext(); + context.setParent(host); + + manager.setContext(context); + + manager.setMaxActiveSessions(2); + manager.setMinIdleSwap(0); + + manager.start(); + + // Create the maximum number of sessions + manager.createSession(null); + manager.createSession(null); + + // Given the minIdleSwap settings, this should swap one out to get below + // the limit + manager.processPersistenceChecks(); + Assert.assertEquals(1, manager.getActiveSessions()); + Assert.assertEquals(2, manager.getActiveSessionsFull()); + + manager.createSession(null); + Assert.assertEquals(2, manager.getActiveSessions()); + Assert.assertEquals(3, manager.getActiveSessionsFull()); + } +} Propchange: tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1738039&r1=1738038&r2=1738039&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Apr 6 20:07:05 2016 @@ -107,6 +107,12 @@ and <code>host</code> attributes define as <code>TransientAttribute</code>. (kfujino) </fix> + <fix> + <bug>59269</bug>: Correct the implementation of + <code>PersistentManagerBase</code> so that <code>minIdleSwap</code> + functions as designed and sessions are swapped out to keep the active + session count below <code>maxActiveSessions</code>. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> Modified: tomcat/trunk/webapps/docs/config/manager.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/manager.xml?rev=1738039&r1=1738038&r2=1738039&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/config/manager.xml (original) +++ tomcat/trunk/webapps/docs/config/manager.xml Wed Apr 6 20:07:05 2016 @@ -215,21 +215,21 @@ </attribute> <attribute name="maxIdleSwap" required="false"> - <p>The time interval (in seconds) since the last access to a session - before it should be persisted to the session store, and - passivated out of the server's memory, or <code>-1</code> to disable - this feature. If this feature is enabled, the time interval specified - here should be equal to or longer than the value specified for - <code>maxIdleBackup</code>. By default, this feature is disabled.</p> + <p>The maximum time a session may be idle before it is eligible to be + swapped to disk due to inactivity. Setting this to <code>-1</code> means + sessions should not be swapped out just because of inactivity. If this + feature is enabled, the time interval specified here should be equal to + or longer than the value specified for <code>maxIdleBackup</code>. By + default, this feature is disabled.</p> </attribute> <attribute name="minIdleSwap" required="false"> - <p>The time interval (in seconds) since the last access to a session - before it will be eligible to be persisted to the session store, and - passivated out of the server's memory, or <code>-1</code> for this - swapping to be available at any time. If specified, this value should - be less than that specified by <code>maxIdleSwap</code>. By default, - this value is set to <code>-1</code>.</p> + <p>The minimum time in seconds a session must be idle before it is + eligible to be swapped to disk to keep the active session count below + maxActiveSessions. Setting to <code>-1</code> means sessions will not be + swapped out to keep the active session count down. If specified, this + value should be less than that specified by <code>maxIdleSwap</code>. + By default, this value is set to <code>-1</code>.</p> </attribute> <attribute name="processExpiresFrequency" required="false"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org