1

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.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
simpatico
  • 10,709
  • 20
  • 81
  • 126
  • 1
    Check the database query log, or start making your own logs, to see what is going wrong. No database will never do rollbacks because of two concurrent users, you probably have errors in your queries or missings commits. – Frank Heikens Jun 05 '10 at 07:52

3 Answers3

6

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 already provides a connection pool feature 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. As a completely different alternative, learn EJB and JPA. It will abstract away all the JDBC boilerplate for you into oneliners.

Hope this helps.

See also:

Community
  • 1
  • 1
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • 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 Jun 05 '10 at 18:40
  • 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 Jun 05 '10 at 18:50
  • Opening and closing connection in shortest possible scope (in the very same method block) should already fix the problem of connections being shared. – BalusC Jun 05 '10 at 19:00
  • 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 Jun 05 '10 at 19:13
  • No, opening the connection by `DriverManager#getConnection()` or `DataSource#getConnection()`, not by `SomeBadSingleTon#getConnection()`. Follow the article link and carefully read it. – BalusC Jun 05 '10 at 21:07
  • 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 Jun 05 '10 at 21:56
  • 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 Jun 05 '10 at 22:17
  • 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 Jun 05 '10 at 22:30
  • 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 Jun 05 '10 at 22:38
  • 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 Jun 05 '10 at 22:49
  • 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 Jun 05 '10 at 23:03
  • 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 Jun 05 '10 at 23:08
  • For that you actually need an extra *transaction* layer, but you at least got the point now. – BalusC Jun 05 '10 at 23:12
1

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
  • 33,309
  • 15
  • 79
  • 114
  • 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 Jun 05 '10 at 18:36
0

Singleton should be the JNDI pool connection itself; Database class with getConnection(), query methods et al should NOT be singleton, but can be static if you prefer.

In this way the pool exists indefinitely, available to all users, while query blocks use dataSource.getConnection() to draw a connection from the pool; exec the query, and then close statement, result set, and connection (to return it to the pool).

Also, JNDI lookup is quite expensive, so it makes sense to use a singleton in this case.

virtualeyes
  • 11,147
  • 6
  • 56
  • 91