I've not had a chance to test these, but this would appear to keep almost all substantive effects out of the base classes and limit them to the session passivation classes which need these changes. I also managed to eliminate the need for the endAccess() override (which was really a workaround for the unbalanced endAccess() call in swapIn() -- though other unbalanced calls might benefit from some debug code in endAccess() as well...).
-- Jess Holle
Jess Holle wrote:
On further analysis it seems that StandardSession is only constructed by ManagerBase and indirectly from SimpleTcpReplicationManager via the ReplicatedSession subclass.
Given that SimpleTcpReplicationManager is mutually exclusive of PersistentManager, it would appear I should move my StandardSession changes into a new PersistentSession class and move my ManagerBase changes into PersistentManagerBase.
Which portions of my changes woulud be objectionable in this case? [Yes, the Request change would still be necessary to prevent the chance of requests randomly getting their sessions passivated and recycle after they findSession() but before access().]
-- Jess Holle
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Index: PersistentManagerBase.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java,v
retrieving revision 1.25
diff -u -r1.25 PersistentManagerBase.java
--- PersistentManagerBase.java 22 Nov 2004 16:35:18 -0000 1.25
+++ PersistentManagerBase.java 17 Dec 2004 05:53:42 -0000
@@ -20,6 +20,11 @@
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
+import java.util.Map;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
@@ -204,17 +209,44 @@
protected long processingTime = 0;
- // ------------------------------------------------------------- Properties
+ /**
+ * Whether session should be synchronously swapped out immediately upon
+ * exceeding maxActiveSessions. Defaults to 'false'.
+ */
+ protected boolean immediateSwapOnMaxActiveExceeded = false;
+
+
+ // -------------------------------------------------------------
Constructors
+
+
+ public PersistentManagerBase() {
+ sessions = new LinkedHashMap() {
+ protected boolean removeEldestEntry(Map.Entry eldest) {
+ if ( immediateSwapOnMaxActiveExceeded ) {
+ int maxActiveSessions = getMaxActiveSessions();
+ if ( maxActiveSessions >= 0 )
+ if ( size() > maxActiveSessions )
+ if ( isStarted() )
+ // try to swap out oldest entry; if entry is
still too fresh then processMaxActiveSwaps() will swap out sessions when
possible in the background
+ swapOutSessionIfOldEnough( (StandardSession)
eldest.getValue(), System.currentTimeMillis(), minIdleSwap,
"persistentManager.swapTooManyActive" );
+ }
+ return ( false );
+ }
+ };
+ }
-
+ // ------------------------------------------------------------- Properties
+
+
+
/**
- * Indicates how many seconds old a session can get, after its last use
in a
- * request, before it should be backed up to the store. -1 means
sessions
- * are not backed up.
- */
+ * Indicates how many seconds old a session can get, after its last use in
a
+ * request, before it should be backed up to the store. -1 means sessions
+ * are not backed up.
+ */
public int getMaxIdleBackup() {
return maxIdleBackup;
@@ -314,13 +346,13 @@
/**
- * Set the Container with which this Manager has been associated. If it
is a
- * Context (the usual case), listen for changes to the session timeout
- * property.
- *
- * @param container
- * The associated Container
- */
+ * Set the Container with which this Manager has been associated. If it is
a
+ * Context (the usual case), listen for changes to the session timeout
+ * property.
+ *
+ * @param container
+ * The associated Container
+ */
public void setContainer(Container container) {
// De-register from the old Container (if any)
@@ -500,6 +532,23 @@
}
+ public boolean isImmediateSwapOnMaxActiveExceeded() {
+ return immediateSwapOnMaxActiveExceeded;
+ }
+
+
+
+ public void setImmediateSwapOnMaxActiveExceeded( boolean
immediateSwapOnMaxActiveExceeded ) {
+ if ( this.immediateSwapOnMaxActiveExceeded ==
immediateSwapOnMaxActiveExceeded )
+ return;
+ boolean oldImmediateSwapOnMaxActiveExceeded =
this.immediateSwapOnMaxActiveExceeded ;
+ this.immediateSwapOnMaxActiveExceeded =
immediateSwapOnMaxActiveExceeded ;
+ support.firePropertyChange("immediateSwapOnMaxActiveExceeded ",
+ new
Boolean(oldImmediateSwapOnMaxActiveExceeded ),
+ new
Boolean(this.immediateSwapOnMaxActiveExceeded ));
+ }
+
+
// --------------------------------------------------------- Public Methods
@@ -534,8 +583,8 @@
/**
* Implements the Manager interface, direct call to processExpires and
processPersistenceChecks
*/
- public void processExpires() {
-
+ public void processExpires() {
+
long timeNow = System.currentTimeMillis();
Session sessions[] = findSessions();
int expireHere = 0 ;
@@ -556,8 +605,8 @@
if(log.isDebugEnabled())
log.debug("End expire sessions " + getName() + " processingTime "
+ (timeEnd - timeNow) + " expired sessions: " + expireHere);
processingTime += (timeEnd - timeNow);
-
- }
+
+ }
/**
@@ -609,6 +658,59 @@
}
/**
+ * Get new session class to be used in the doLoad() method.
+ */
+ protected StandardSession getNewSession() {
+ return new PersistentSession(this);
+ }
+
+ /** If upon accessing a session we find it initially invalid see if it was
+ * just swapped out. If so, attempt to read the data back into this
+ * session object as someone got a copy prior to the swapOut.
+ *
+ * @param session Session that is being accessed.
+ */
+ protected void preAccess( Session session )
+ {
+ PersistentSession persistentSession = (PersistentSession) session;
+ if ( persistentSession.isValid || persistentSession.expiring )
+ return;
+ String id = persistentSession.getId();
+ StandardSession savedSession = null;
+ try
+ {
+ savedSession = (StandardSession) swapInWithoutAdd( id );
+ }
+ catch ( IOException e )
+ {
+ if (log.isDebugEnabled())
+ log.debug("Load from store failed for " + id );
+ }
+ if ( savedSession != null )
+ {
+ persistentSession.copyFieldsOf( savedSession );
+ addSwappedInSession( persistentSession );
+ savedSession.recycle();
+ }
+ }
+
+ /**
+ * Signal to Manager that session has been accessed.
+ * <p>
+ * This is used here to maintain MRU/LRU data on sessions.
+ *
+ * @param session Session that is being accessed.
+ */
+ protected void access( Session session ) {
+ synchronized (sessions) {
+ // pull session out of map and put it back in, thus sorting it
+ String sid = session.getId();
+ sessions.remove(sid);
+ sessions.put(sid, session);
+ }
+ }
+
+ /**
* Load all sessions found in the persistence mechanism, assuming
* they are marked as valid and have not passed their expiration
* limit. If persistence is not supported, this method returns
@@ -660,6 +762,39 @@
log.error("Failed load session from store, " + e.getMessage(),
e);
}
+ sortSessionsByAccessTimes();
+ }
+
+
+ /** Sort sessions by access times, i.e. during load().
+ */
+ private void sortSessionsByAccessTimes() {
+ ArrayList sessionList;
+ {
+ Session sessionArr[] = findSessions();
+ sessionList = new ArrayList( sessionArr.length );
+ for ( int i = 0; i < sessionArr.length; ++i )
+ sessionList.add( sessionArr[i] );
+ }
+ Collections.sort( sessionList,
+ new Comparator() {
+ public int compare(Object o1, Object o2) {
+ StandardSession session1 = (StandardSession) o1;
+ StandardSession session2 = (StandardSession) o2;
+ long accessTime1 = session1.thisAccessedTime;
+ long accessTime2 = session2.thisAccessedTime;
+ if ( accessTime1 < accessTime2 )
+ return -1;
+ if ( accessTime1 == accessTime2 )
+ return 0;
+ // if ( accessTime1 > accessTime2 )
+ return 1;
+ }
+ }
+ );
+ int nSessions = sessionList.size();
+ for ( int i = 0; i < nSessions; ++i )
+ access( (Session) sessionList.get( i ) );
}
@@ -749,6 +884,13 @@
* is invalid or past its expiration.
*/
protected Session swapIn(String id) throws IOException {
+ Session session = swapInWithoutAdd( id );
+ if ( session != null )
+ addSwappedInSession( session );
+ return ( session );
+ }
+
+ protected Session swapInWithoutAdd(String id) throws IOException {
if (store == null)
return null;
@@ -791,15 +933,21 @@
if(log.isDebugEnabled())
log.debug(sm.getString("persistentManager.swapIn", id));
+ return (session);
+
+ }
+
+ protected void addSwappedInSession( Session session )
+ {
+ StandardSession standardSession = (StandardSession) session;
session.setManager(this);
// make sure the listeners know about it.
- ((StandardSession)session).tellNew();
+ standardSession.tellNew();
add(session);
- ((StandardSession)session).activate();
- session.endAccess();
-
- return (session);
-
+ standardSession.activate();
+ // next two lines were done via endAccess() except this left negative
accessCount()!
+ standardSession.isNew = false;
+ standardSession.accessCount = 0;
}
@@ -1023,29 +1171,10 @@
long timeNow = System.currentTimeMillis();
// Swap out all sessions idle longer than maxIdleSwap
- // FIXME: What's preventing us from mangling a session during
- // a request?
- if (maxIdleSwap >= 0) {
- for (int i = 0; i < sessions.length; i++) {
- StandardSession session = (StandardSession) sessions[i];
- if (!session.isValid())
- continue;
- int timeIdle = // Truncate, do not round up
- (int) ((timeNow - session.getLastAccessedTime()) / 1000L);
- if (timeIdle > maxIdleSwap && timeIdle > minIdleSwap) {
- if (log.isDebugEnabled())
- log.debug(sm.getString
- ("persistentManager.swapMaxIdle",
- session.getId(), new Integer(timeIdle)));
- try {
- swapOut(session);
- } catch (IOException e) {
- ; // This is logged in writeSession()
- }
- }
- }
- }
-
+ int minSwapSecs = ( ( maxIdleSwap > minIdleSwap ) ? maxIdleSwap :
minIdleSwap );
+ for (int i = sessions.length - 1; i >= 0; --i)
+ swapOutSessionIfOldEnough( (StandardSession) sessions[i], timeNow,
minSwapSecs, "persistentManager.swapMaxIdle" );
+
}
@@ -1059,7 +1188,6 @@
Session sessions[] = findSessions();
- // FIXME: Smarter algorithm (LRU)
if (getMaxActiveSessions() >= sessions.length)
return;
@@ -1071,24 +1199,36 @@
int toswap = sessions.length - getMaxActiveSessions();
long timeNow = System.currentTimeMillis();
- for (int i = 0; i < sessions.length && toswap > 0; i++) {
- int timeIdle = // Truncate, do not round up
- (int) ((timeNow - sessions[i].getLastAccessedTime()) / 1000L);
- if (timeIdle > minIdleSwap) {
- if(log.isDebugEnabled())
- log.debug(sm.getString
- ("persistentManager.swapTooManyActive",
- sessions[i].getId(), new Integer(timeIdle)));
- try {
- swapOut(sessions[i]);
- } catch (IOException e) {
- ; // This is logged in writeSession()
- }
- toswap--;
- }
- }
+ for (int i = sessions.length - 1; i >= 0 && toswap > 0; --i)
+ if ( swapOutSessionIfOldEnough( (StandardSession) sessions[i],
timeNow, minIdleSwap, "persistentManager.swapTooManyActive" ) )
+ --toswap;
}
+
+
+ private boolean swapOutSessionIfOldEnough( StandardSession session, long
timeNow, int minSwapSecs, String debugString ) {
+ synchronized ( session ) {
+ int timeIdle = // Truncate, do not round up
+ (int) ((timeNow - session.thisAccessedTime) / 1000L);
+ if (timeIdle > minSwapSecs ) {
+ if ( session.accessCount > 0 ) {
+ if (log.isDebugEnabled())
+ log.debug(
"PersistentManagerBase.swapOutSessionIfOldEnough(): session [" +
session.getId() +
+ "] access count [" + session.accessCount + "]
> 0 " );
+ return ( false ); // session may still be being accessed
+ }
+ if(log.isDebugEnabled())
+ log.debug(sm.getString(debugString, session.getId(), new
Integer(timeIdle)));
+ try {
+ swapOut(session);
+ } catch (IOException e) {
+ ; // This is logged in writeSession()
+ }
+ return ( true );
+ }
+ }
+ return ( false );
+ }
/**
@@ -1104,22 +1244,32 @@
// Back up all sessions idle longer than maxIdleBackup
if (maxIdleBackup >= 0) {
- for (int i = 0; i < sessions.length; i++) {
+ for (int i = sessions.length - 1; i >= 0; --i) {
StandardSession session = (StandardSession) sessions[i];
- if (!session.isValid())
- continue;
- int timeIdle = // Truncate, do not round up
- (int) ((timeNow - session.getLastAccessedTime()) / 1000L);
- if (timeIdle > maxIdleBackup) {
- if (log.isDebugEnabled())
- log.debug(sm.getString
- ("persistentManager.backupMaxIdle",
- session.getId(), new Integer(timeIdle)));
-
- try {
- writeSession(session);
- } catch (IOException e) {
- ; // This is logged in writeSession()
+ synchronized ( session )
+ {
+ if (!session.isValid())
+ continue;
+ int timeIdle = // Truncate, do not round up
+ (int) ((timeNow - session.thisAccessedTime) / 1000L);
+ if (timeIdle > maxIdleBackup) {
+ if ( session.accessCount > 0 ) {
+ if (log.isDebugEnabled())
+ log.debug(
"PersistentManagerBase.processMaxIdleBackups(): session [" + session.getId() +
+ "] access count [" +
session.accessCount + "] > 0 " );
+ continue; // session may still be being accessed
+ }
+
+ // TO_DO: enhance store to allow quick
'if-modified-since' sort of check here!
+ if (log.isDebugEnabled())
+ log.debug(sm.getString
+ ("persistentManager.backupMaxIdle",
+ session.getId(), new Integer(timeIdle)));
+ try {
+ writeSession(session);
+ } catch (IOException e) {
+ ; // This is logged in writeSession()
+ }
}
}
}
package org.apache.catalina.session;
import java.util.HashMap;
import org.apache.catalina.Manager;
/**
* Subclass of StandardSession used by PersistentManagerBase.
* @author Jess Holle
*/
public class PersistentSession
extends StandardSession
{
public PersistentSession( Manager manager ) {
super( manager );
}
/**
* Overrides StandardSession.access() to allow pre-access correct and
* post-access book keeping by PersistentManagerBase.
*/
public synchronized void access() {
if ( manager instanceof PersistentManagerBase )
((PersistentManagerBase)manager).preAccess( this );
super.access();
if ( !isValid )
if ( manager instanceof PersistentManagerBase )
((PersistentManagerBase)manager).access( this );
}
/**
* Like StandardSession.recycle() except here the id and manager are
* preserved in order for PersistentManagerBase.preAccess() to operate
* properly.
*/
public void recycle() {
String id = this.id;
Manager manager = this.manager;
super.recycle();
this.id = id;
this.manager = manager;
}
/**
* Copy all non-transient fields other than id from 'otherSession' to
* this session. Not very elegant but necessary in conjunction with
* PersistentManagerBase.preAccess() to resurrect in-use sessions which
* get passivated and recycled due to a race condition.
*/
protected void copyFieldsOf( StandardSession otherSession )
{
this.attributes = (HashMap) otherSession.attributes.clone();
this.creationTime = otherSession.creationTime ;
this.lastAccessedTime = otherSession.lastAccessedTime;
this.maxInactiveInterval = otherSession.maxInactiveInterval;
this.isNew = otherSession.isNew;
this.isValid = otherSession.isValid;
this.thisAccessedTime = otherSession.thisAccessedTime;
}
}
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
