Bug 34319

Summary: StoreBase.processExpires() is very inefficient
Product: Tomcat 9 Reporter: Tom Anderson <tom>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Severity: enhancement CC: grant-genereux
Priority: P2    
Version: 9.0.0.M1   
Target Milestone: -----   
Hardware: All   
OS: All   
Attachments: JDBCStore example

Description Tom Anderson 2005-04-06 04:12:25 UTC
Using a session store such as JDBCStore with the default configuration 
(Engine.backgroundProcessorDelay=10 and StandardManager.processExpiresFrequency=6), 
StoreBase.processExpires() will wake up once a minute and load all stored sessions into memory.   So, 
with the default 30-minute session-timeout setting, this means we are loading up to 29 minutes worth 
of sessions that won't be ready to expire.   In this example we load 30x more sessions than we actually 
need to. 

Since the purpose of StoreBase.processExpires() is to find sessions that are ready to be expired, it 
doesn't make sense to load ALL session for cases where we could pre-filter sessions BEFORE loading 
them.   For example, JDBCStore could use a where clause to reduce the number of sessions loaded as 
candidates to be expired and FileStore could query the filesystem for the age of the files before loading.    
The current behavior could be the worst-case fallback for Stores in which it is not possible to get age 

I propose that StoreBase.processExpires() be refactored to load only keys of old sessions when possible 
by providing an oldKeys() method that may be overriden.   By default this would just return all keys but 
it should be easy to extend this in specialized versions (I will provide a sample patch to illustrate for 
Comment 1 Tom Anderson 2005-04-06 04:14:57 UTC
Created attachment 14627 [details]
JDBCStore example

This is just one way of doing it.   In any case I'd be happy with any
alternative that achieves the same goal.
Comment 2 Tom Anderson 2005-04-07 22:49:59 UTC
Created attachment 14654 [details]

Here's a better idea.	Instead of loading in any sessions at all, why not
override processExpires() to simply remove the old sessions.   This
accomplishes exactly the same thing without all of the loading and deleting of
sessions.   This is a huge performance (and memory usage) improvment.
Comment 3 Mark Thomas 2006-09-11 02:52:33 UTC
Changing the severity to enhancement and updating the version.

I have only skimmed the patches but the first one looks plausible. The approach
of the second bypasses the listener notification and therefore would not work.
Comment 4 Yoav Shapira 2006-12-24 08:11:07 UTC
Closing this as WONTFIX given how much time has passed with no activity, votes,
or comments on the issue.  If the original poster still cares enough to submit a
patch that does not bypass the listener, he can reopen this issue and attach
said patch at that time.
Comment 5 Felix Schumacher 2015-11-21 13:18:24 UTC
*** Bug 47281 has been marked as a duplicate of this bug. ***
Comment 6 Mark Thomas 2015-11-21 18:47:34 UTC
Re-opening for fixing in a latter version.
Comment 7 Mark Thomas 2016-02-07 19:25:27 UTC
A variation of this patch was applied to 9.0.x for 9.0.0.M3 onwards.