views:

65

answers:

2

I have a severe problem with my database connection in my web application. Since I use a single database connection for the whole application from singleton Database class, if i try concurrent db operations (two users) the database rollsback the transactions. This is my static method used:

All threads/servlets call static Database.doSomething(...) methods, which in turn call the the below method.

private static /* synchronized*/ Connection getConnection(final boolean autoCommit) throws SQLException {
    if (con == null) {
        con = new MyRegistrationBean().getConnection();
    }
    con.setAutoCommit(true); //TODO
    return con;
}

What's the recommended way to manage this db connection/s I have, so that I don't incurr in the same problem.

+1  A: 

I've not much experience with PostgreSql, but all the web applications I've worked on have used a single connection per set of actions on a page, closing it and disposing it when finished.

This allows the server to pool connections and stops problems such as the one that you are experiencing.

Paddy
Yes, but isn't a new servlet thread created for each client request? Then if I have a connection per servlet (closed at the end of it), I'll soon receive a: fatal error: too many connections already. Isn't it?
simpatico
+3  A: 

Keeping a Connection open forever is a very bad idea. It doesn't have an endless lifetime, your application may crash whenever the DB times out the connection and closes it. Best practice is to acquire and close Connection, Statement and ResultSet in the shortest possible scope to avoid resource leaks and potential application crashes caused by the leaks and timeouts.

Since connecting the DB is an expensive task, you should consider using a connection pool to improve connecting performance. A decent applicationserver/servletcontainer usually provides a connection pooling facility in flavor of a JNDI DataSource. Consult its documentation for details how to create it. In case of for example Tomcat you can find it here.

Even when using a connection pool, you still have to write proper JDBC code: acquire and close all the resources in the shortest possible scope. The connection pool will on its turn worry about actually closing the connection or just releasing it back to pool for further reuse.

You may get some more insights out of this article how to do the JDBC basics the proper way.

Hope this helps.

BalusC
for simplicity (now), I'm ok with the time it takes to open and close the connection. Actually I'm not even (yet) worried about the crash after the DB timeout. My question is about synchronizing the multiple requests of a connection to the db. My method above doesn't do any synchronization. I'm not sure what's happening, but it seems like both threads share the same connection and that's detected as a problem in itself, or the 2nd thread steels it from the 1st, again being detected as an error.
simpatico
and btw, I was already closing the connection at the end of each db method, but that conflicts with this singleton connection, because one thread could close it before another is done with it.
simpatico
Opening and closing connection in shortest possible scope (in the very same method block) should already fix the problem of connections being shared.
BalusC
how?! "I was already closing the connection at the end of each db method, but that conflicts with this singleton connection, because one thread could close it before another is done with it."Are you considering that my Database has only static methods, and so thread A could be in method m (with the connection) while thread B comes and calls method m too! (or m' if u like) and when 2nd m requests the connection I get my problem.
simpatico
No, opening the connection by `DriverManager#getConnection()` or `DataSource#getConnection()`, not by `SomeBadSingleTon#getConnection()`. Follow the article link and carefully read it.
BalusC
I extract the relevant bits: Connection getConnection(){ return dataSource.getConnection(); }This is just what my new MyRegistrationBean().getConnection();, i.e. return DriverManager.getConnection("jdbc:jdc:jdcpool");new or an instance variable, is the same thing since MyRegistrationBean includes:static { try { new MyConnectionDriver(className, url, username, password); }So the only real difference btw my code and the one you suggest (in these bits) is not keeping a singleton connection, and static. That didn't work. How could static db class be the problem?
simpatico
You told you were using a single connection for the whole application. That's the problem. You should create a physically new connection inside the very same method block where you're doing the query and close it in the `finally`.
BalusC
yes, because doing that results in FATAL: sorry, too many clients already authentication failed. For the nth trial, I've changed the code so that the getConnection stated in the q, returns new MyRegistrationBean().getConnection(), instead of keeping a singleton con, and return that. That's a new connection for each method, isn't it? Then I do close it with code very similar to yours.
simpatico
That's correct. I still strongly recommend to read the last linked article in my answer to learn how to do this all nicely.
BalusC
I did read it. Indeed from it I've caught the nice idea of a close function, and made the above observations. I'm willing to plugin all that DAO code though (maybe in some other new project), because it's beyond my purposes. Besides, that wouldn't explain the problem of my problem. According to your claims I shouldn't be getting this FATAL error, yet I'm. The only smelly thing left is static, and that I get the connection from within db class, not the servlet (possibly on another thread).
simpatico
The initial problem was that you're returning the **same** connection everytime. You create it only once and returns it everytime. That's just plain wrong. As to the *too many clients* error, the detail/stacktrace is missing, but this just sounds like as if that the opened connections are not closed properly. Fixing the code to make it close them all in finally and restarting the app and the DB should resolve it.
BalusC
okay,let me get it right. restart : sure.I'm indeed not doing finally everywhere, because that would mean that my servlets don't call Database.m(..) but need to keep Connection and pass it along. Or add in between a middleman (no!), or use con as static variables in database -> will not work. So will try with passing Connection along from servlet to Database.m(..) and finally in servlet code.
simpatico
For that you actually need an extra *transaction* layer, but you at least got the point now.
BalusC