My team has to make some changes and renew an old web application. This application has one main thread and 5 to 15 daemon threads used as workers to retrieve and insert data in a DB.
All those threads have this design (here simplified for convenience):
public MyDaemon implements Runnable {
// initialization and some other stuffs
public void run() {
...
while(isEnabled) {
Engine.doTask1();
Engine.doTask2();
...
Thread.sleep(someTime);
}
}
}
The Engine class provides a series of static methods used to maipulate other methods of DataAccessor classes, some of those methods been static:
public Engine {
public static doTask1() {
ThisDataAccessor.retrieve(DataType data);
// some complicated operations
ThisDataAccessor.insertOrUpdate(DataType data);
}
public static doTask2() {
ThatDataAccessor da = new ThatDataAccessor();
da.retrieve(DataType data);
// etc.
}
...
}
DataAccessor classes usually interact with DB using simple JDBC statements enclosed in synchronized methods (static for some classes). DataSource is configured in the server.
public ThatDataAccessor {
public synchronized void retrieve(DataType data) {
Connection conn = DataSource.getConnection();
// JDBC stuff
conn.close();
}
...
}
The problem is that the main thread needs to connect to DB and when these daemon threads are working we run easily out of available connections from the pool, getting "waiting for connection timeout" exceptions. In addition, sometimes even those daemon threads get the same exception.
We have to get rid of this problem.
We have a connection pool configured with 20 connections, and no more can be added since that "20" is our production environment standard. Some blocks of code need to be synchronized, even if we plan to move the "synchronized" keyword only where really needed. But I don't think that it would make really the difference.
We are not experienced in multithreading programming and we've never faced this connection pooling problem before, that's why I'm asking: is the problem due to the design of those threads? Is there any flaw we haven't noticed?
I have profiled thread classes one by one and as long as they are not running in parallel it seems that there's no bottleneck to justify those "waiting for connection timeout". The app is running on WebSphere 7, using Oracle 11g.
You are likely missing a finally block somewhere to return the connections back to the pool. With hibernate, I think this is probably done when you call close() or possibly for transactions, when you call rollback(). But I would call close anyway.
For example, I wrote a quick and dirty pool myself to extend an old app to make it multithreaded, and here is some of the handling code (which should be meaningless to you except the finnally block):
try {
connection = pool.getInstance();
connection.beginTransaction();
processFile(connection, ...);
connection.endTransaction();
logger_multiThreaded.info("Done processing file: " + ... );
} catch (IOException e) {
logger_multiThreaded.severe("Failed to process file: " + ... );
e.printStackTrace();
} finally {
if (connection != null) {
pool.releaseInstance(connection);
}
}
It is fairly common for people to fail to use finally blocks properly... For example, look at this hibernate tutorial, and skip to the very bottom example. You will see that in the try{} he uses tx.commit() and in the catch{} he uses tx.rollback(), but he has no session.close(), and no finally. So even if he added a "session.close()" in try and in catch, if his try block threw something other than a RuntimeException, or his catch caused an additional Exception before the try or a non-HibernateException before the rollback(), his connection would not be closed. And without session.close(), I don't think that is actually very good code. But even if the code is seemingly working, a finally gives you assurance that you are protected from this type of problem.
So I would rewrite his methods that use Session to match the idiom shown on this hibernate documentation page. (and also I don't recommend his throwing a RuntimeException, but that is a different topic).
So if you are using Hibernate, I think the above is good enough. But otherwise, you'll need to be more specific if you want specific code help, but otherwise the simple idea that you should use a finally to ensure the connection is closed is enough.