2012/11/5 ken dombeck <ken_domb...@yahoo.com>: > Occasionally when a user signs into our web site it is taking a very long > time to create a session (10 to 100+ seconds). We traced the issue do to the > fact that the background process (ContainerBackgroundProcessor) calls > PersistentManagerBase.processExpires() to swap out sessions from memory and > remove expired sessions from the database. The issue only appears when we > have 10’s of thousands of sessions in memory/database. > > The problem is that even though StoreBase.processExpires() correctly releases > the locks from JDBCStore (load and remove methods) JDK 6 has implemented > “bias locking” > http://www.oracle.com/technetwork/java/tuning-139912.html#section4.2.5 which > causes this process to ‘hold’ onto the locks obtained in JDBCStore until it > is completely done. > > It is this thread starvation, when the background process > (ContainerBackgroundProcessor) expires/swaps out session runs that other > threads are prevented from load sessions from the database due to the locks > obtained in JDBCStore. > > We are running Tomcat 6.0, OpenJDK 6 (same problem with OpenJDK 7), Ubuntu > 10.04. > > The problem appears to be a JDK issue that is explained very well in this > blog post. > http://ceki.blogspot.com/2009/06/biased-locking-in-java-se-60.html > > Reading the blog and looking at the JDK bug tracker it does not appear that > the issue will be fixed either. > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4985566 > > We have come up with a few solutions to this issue that work and would like > the community’s input prior to submitting a patch. > > 1) Connection Pool - currently in Tomcat 6.0 JDBCStore only takes a > connectionURL. In Tomcat 7.0 this has been changed to be a connectionURL or > dataSourceName. If connectionURL were to be removed from this class and a > connection pool were to be used for dataSourceName, then the synchronization > could be completely removed from this class. > > Pros: Completely removing the global synchronization improves concurrency > which is moved to the JDBC pool implementation instead. > Cons: The main issue with this solution is that it would affect all existing > configurations that use connectionURL. > > 2) Thread priority - By allowing a user to specify the priority of the > background process (ContainerBackgroundProcessor) to be a lower priority than > other threads the JVM will allow those other threads to obtain the lock > occasionally. This did not work as well as the other solutions since the JVM > did not relinquish control from the background thread to the other worker > threads as often as we would have liked. > > Pros: This would solve any synchronization issues other than just JDBCStore > for the entire background process. > Cons: With the background thread set to a lower priority the other worker > threads only obtained a time slice ever 3 seconds unlike milliseconds for the > other implementations. Thread priority could also be dependent on the JVM > implementation/platform. > > 3) Fair lock - By replacing the synchronization blocks in JDBCStore with > java.util.concurrent.locks.ReentrantLock and constructing the lock with > fair=true we would allow each thread to get the appropriate time slice. > > Pros: This could be written in a way that would default to fair=false to > maintain the default behaviour. A configuration parameter could be added to > allow for the setting of the fair flag. > Cons: May be a small performance penalty. > > Our proposal is solution #3. > > Let me know your thoughts and we will supply a patch.
1) I'd prefer #1, though it might be easier to implement a separate class rather than patch this one (like JDBCRealm vs DataSourceRealm). Currently there are a number of fields in JDBCStore that cannot be used by more than one thread (e.g. JDBCStore#preparedKeysSql and other PreparedStatement s there). Such changes will change API, so it is better to write a separate class. Disclamer: I have not looked whether such multithreading makes sense for an org.apache.catalina.Store. I have not reviewed that API. 2) BTW, I wonder whether the following pattern in JDBCStore synchronized (this) { Connection _conn = getConnection(); } may be responsible for some "Sessions are not expiring" threads that I see on the users list. E.g. http://markmail.org/thread/jkbbiqpxodefpm6i (Though we are still waiting for a thread dump in that thread, so currently it is not known what the problem is). A common mistake with DBCP pool is to configure an infinite timeout there. 3) Have you tested that #3 solves your problem? IMO, changing synchronization object is a slight API change (from the POV of child classes). I would not object to it though if it solves a real problem and the lock object is available to the child classes. 4) It is possible to configure the background thread to run more rarely. It is also possible to configure separate background thread per container. You can have a separate thread for your webapp. It is also possible to configure "processExpiresFrequency" attribute on a <Manager>. 5) I wonder whether it is possible to implement expiration in JDBCStore more effectively. Couldn't expired sessions be selected with one SQL statement, instead of iterating over the all of them and loading them one-by-one? That is what the default implementation in StoreBase#processExpires() does. There are already separate "sessionLastAccessedCol" and "sessionMaxInactiveCol" columns in the JDBCStore configuration. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org