tomcat-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: Thread starvation with JDBCStore
Date Tue, 06 Nov 2012 17:24:42 GMT
2012/11/5 ken dombeck <ken_dombeck@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org


Mime
View raw message