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

Reply via email to