Bug 47281

Summary:	Efficiency of the JDBCStore
Product:	Tomcat 6	Reporter:	Grant Genereux <grant-genereux>
Component:	Catalina	Assignee:	Tomcat Developers Mailing List <dev>
Status:	RESOLVED DUPLICATE
Severity:	enhancement
Priority:	P2
Version:	6.0.18
Target Milestone:	default
Hardware:	All
OS:	All

Description Grant Genereux 2009-05-28 05:38:13 UTC

I’ve been using the PersistentManager with the JDBCStore. On the surface it appears to work as designed. However, depending on how it is used, I think it is somewhat self-defeating.

If it is being used to simply save and restore sessions across application restarts, then I think it works fine.

If you want to use it to swap out idle sessions and reduce the total amount of memory theses idle session are consuming, then it is not too effective.

The issues is with in the processExpires method of org.apache.catalina.session.StoreBase and called from org.apache.catalina.session.PersistentManagerBase.

Basically speaking; what StoreBase.processExpires() does is

1. Gets all the keys from the store ( in this case the JDBCStore)
2. Loads all the sessions from the JDBCStore
3. Removes and deletes any expired session.

The problem is that in step 2 above, we’ve loaded all the fully de-serialized sessions back into memory. This happens every few minutes with the default configuration.

So, at the time of the call to processExpires() the total amount of memory being consumed by sessions is no less than if we had not swapped them out to the store.

It is actually more if we are using the persistent store to back up ( not swap out) sessions. Since, in that case, we may now have two instances of each of these sessions in memory.

I’ve implemented a different method to delete expired sessions directly from the DB, without reloading them into memory.

Effectively executing a SQL statement like:

DELETE FROM tomcat_sessions WHERE app_name = ? AND ( valid_session = '0' OR ? > (last_access+max_inactive*1000) )

Where the second parameter is set with the System.currentTimeMillis().

But, there is a downside to this approach, in that the full life cycle contract of the sessions is not observed. No listeners are called when the session is removed from store.

In my application’s case, this is not an issue. But, it most certainly could be for others.

Additionally, it could be that for some reason an expired session swapped out to the store is still in memory. So, we should probably first get the session keys of the expired sessions before we delete them, and ensure that they are removed from memory as well.
I am happy to share my work-around, and work on a more robust solution if anyone is interested.

Comment 1 Mark Thomas 2009-05-28 05:59:47 UTC

(In reply to comment #0)

> Basically speaking; what StoreBase.processExpires() does is
> 
> 1.    Gets all the keys from the store ( in this case the JDBCStore)
> 2.    Loads all the sessions from the JDBCStore
> 3.    Removes and deletes any expired session.
> 
> The problem is that in step 2 above, we’ve loaded all the fully de-serialized
> sessions back into memory.  This happens every few minutes with the default
> configuration.  
> 
> So, at the time of the call to processExpires() the total amount of memory
> being consumed by sessions is no less than if we had not swapped them out to
> the store.  

Are you sure. Looking at the code it loops through the list of sessions doing steps 2 and 3 above for one session at a time rather than all sessions at once.

Comment 2 Grant Genereux 2009-05-28 06:35:44 UTC

(In reply to comment #1)
> (In reply to comment #0)
> 
> > Basically speaking; what StoreBase.processExpires() does is
> > 
> > 1.    Gets all the keys from the store ( in this case the JDBCStore)
> > 2.    Loads all the sessions from the JDBCStore
> > 3.    Removes and deletes any expired session.
> > 
> > The problem is that in step 2 above, we’ve loaded all the fully de-serialized
> > sessions back into memory.  This happens every few minutes with the default
> > configuration.  
> > 
> > So, at the time of the call to processExpires() the total amount of memory
> > being consumed by sessions is no less than if we had not swapped them out to
> > the store.  
> 
> Are you sure. Looking at the code it loops through the list of sessions doing
> steps 2 and 3 above for one session at a time rather than all sessions at once.

Thanks for the quick reply,

Yes, but depending upon when the GC thread kicks in.
If not during the: 
  for (int i = 0; i lt keys.length; i++) 
then by the end of that statement, we'll have all the sessions back in memory.
Yes, since the reference is not kept, the GC can collect them, but it could be a bit intensive.

Thanks

Comment 3 Grant Genereux 2009-05-28 09:13:38 UTC

After some more thought; I think a more correct approach would be to have

StoreBase.processExpires()  call a method to just return the expired keys; rather than keys() (returns all keys).  It would be very similar to keys() except it includes an additional where clause such as:

AND ( valid_session = '0' OR  ?
.gt. (last_access+max_inactive*1000)  )


With that change, only expired sessions will be re-loaded, and we maintain the session life-cycle contract.

Comment 4 Felix Schumacher 2015-11-21 13:18:24 UTC


*** This bug has been marked as a duplicate of bug 34319 ***