Closing JDBC Connections in Pool

Question

Our standard code section for using JDBC is...

Connection conn = getConnection(...);
Statement  stmt = conn.conn.createStatement (ResultSet.TYPE_SCROLL_INSENSITIVE,
                                                ResultSet.CONCUR_READ_ONLY);
ResultSet  rset = stmt.executeQuery (sqlQuery);

// do stuff with rset

rset.close(); stmt.close(); conn.close();

Question 1: When using Connection Pool, should one close the Connection at the end? If so, isn't the purpose of pooling lost? And if not, how does the DataSource know when a particular instance of Connection is freed up and can be reused? I am a little confused on this one, any pointers appreciated.

Question 2: Is the following method anything close to standard? Looks like an attempt to get a connection from the pool, and if DataSource cannot be established, use the old fashioned DriverManager. We are not even sure which part is getting executed at runtime. Repeating the question above, should one close the Connection coming out of such a method?

synchronized public Connection getConnection (boolean pooledConnection)
                                                        throws SQLException {
        if (pooledConnection) {
                if (ds == null) {
                        try {
                                Context envCtx = (Context)
                                        new InitialContext().lookup("java:comp/env");
                                ds = (DataSource) envCtx.lookup("jdbc/NamedInTomcat");
                                return ds.getConnection();
                        } catch (NamingException e) {
                                e.printStackTrace();
                }}
                return (ds == null) ? getConnection (false) : ds.getConnection();
        }
        return DriverManager.getConnection(
                "jdbc:mysql://"+ipaddy+":"+dbPort +"/" + dbName, uName, pWord);
}

Edit: I think we are getting the pooled connection since we do not see a stack trace.

Yam

Answer 1

When using Connection Pool, should one close the Connection at the end? If so, isn't the purpose of pooling lost? And if not, how does the DataSource know when a particular instance of Connection is freed up and can be reused? I am a little confused on this one, any pointers appreciated.

Yes, certainly you need to close the pooled connection as well. It's actually a wrapper around the actual connection. It wil under the covers release the actual connection back to the pool. It's further up to the pool to decide whether the actual connection will actually be closed or be reused for a new getConnection() call. So, regardless of whether you're using a connection pool or not, you should always close all the JDBC resources in reversed order in the finally block of the try block where you've acquired them. In Java 7 this can be further simplified by using try-with-resources statement.

---

Is the following method anything close to standard? Looks like an attempt to get a connection from the pool, and if DataSource cannot be established, use the old fashioned DriverManager. We are not even sure which part is getting executed at runtime. Repeating the question above, should one close the Connection coming out of such a method?

The example is pretty scary. You just need to lookup/initialize the DataSource only once during application's startup in some constructor / initialization of an applicationwide DB config class. Then just call getConnection() on the one and same datasource throughout the rest of application's lifetime. No need for synchronization nor nullchecks.

See also:

• Is it safe to use a static java.sql.Connection instance in a multithreaded system?
• Am I Using JDBC Connection Pooling?

Firelock

Thats what it is doing (initialize once), isnt it? ds is an instance variable, and if (ds == null) ... is the initialization part. – Yam

Doing the checks everytime in a get-method like getConnection() is weird. Just do it in c'tor or initialization block of the very same class, without synchronization/nullchecks. It'll be called only once. For more hints and kickoff examples, you may find this article useful. – Firelock

Excellent article, BalusC. The class I am dealing with pretty much implements the Data Layer, using DTO's. I agree with you, initialization should be in constructor. Now, this class has a ton of methods, each with conn, stmt and rset as local variables, connections are in a try block, and in finally there is a 1-line call csrClose (conn, stmt, rset), where all 3 are closed (in reverse order). Now, the DTO you develop in the example is a mirror image of a DB table row. We have complex SQL queries with joins (and other clauses), do you have an article on how to develop DAO's for such results? – Yam

Not really, actually. There are basically three options: 1) just do it in same method, 2) delegate it to another DAO method with IN clause, 3) use lazy loading in the property getter (like as Hibernate/JPA under the covers does). – Firelock

Eventhough this answer seem to have been accepted and voted up, i see there are difference of opinions between the 3 answers. I was with understanding that we shouldn't close the CONNECTION (taken from connection pool), but close other resources (statement, resultset). SHOULD I or SHOULD NOT ? – Blairblaire

@yat: You MUST call close() on all of them in the finally block of the very same try block as where you acquired/created them. This is completely regardless of if it's a pooled connection or not. – Firelock

I've read you answer. Now I try to start to work with hsqldb and I need a pool. I found only one pool example for this database. sridharrao85.wordpress.com/2011/07/20/… However, as I understand this implementation right if we close connection from pool then pool won't work. As I see there must be used pool.releaseConnection(Connection connection) instead. Can you comment? – Maidenly

@iJava: that pool is written by an amateur who has no utter idea what he's doing. Ignore it and go for a real library. E.g. HikariCP. – Firelock

Thank you that found time to look at. I will try HikariCP and commons-dbcp – Maidenly

It's worth mentioning that calling close() on a PooledConnection will also commit your transaction if AutoCommit is set to true. – Elsyelton

Answer 2

The pools typically return you a wrapped Connection object, where the close() method is overridden, typically returning the Connection to the pool. Calling close() is OK and probably still required.

A close() method would probably look like this:

public void close() throws SQLException {
  pool.returnConnection(this);
}

For your second question, you could add a logger to show whether the bottom block ever runs. I would imagine though you'd only want one way or the other for the configuration of your database connections. We solely use a pool for our database accesses. Either way, closing the connection would be pretty important to prevent leaks.

Energetics

I agree, we do have a logger, and that could be used here, too. I need to study a bit on how you can wrap an Object, override its close() method but still maintain the same class name (Connection) – Yam

Calling close() is OK and probably still required., not calling close will leak the connection, unless the pool implements some recovery strategy – Barca

Answer 3

Actually, the best approach to connection management is to not farm them out to any code anywhere.

Create a SQLExecutor class that is the one and only location which opens and closes connections.

The entire rest of the application then pumps statements into the executor rather than getting connections from the pool and managing (or mismanaging them) all over the place.

You can have as many instances of the executor as you want, but no one should be writing code that opens and closes connections on its own behalf.

Conveniently, this also lets you log all your SQL from a single set of code.

Enalda

This seems like a reasonable idea at least for some applications, but it's also only somewhat related to the question posed here. Presumably you are responding to Question 2 in particular, but even then your prescribed approach does not preclude the use of connections altogether, only outside of this SQLExectutor class. So you could consider the question within the context of your SQLExecutor class. – Jeepers