views:

1017

answers:

2

I have a Java-JSF Web Application on GlassFish, in which I want to use connection pooling. Therefore I created an application scoped bean that serves with Connection instances for other beans:

import java.sql.Connection;
import java.sql.SQLException;
import javax.sql.DataSource;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;

public class DatabaseBean {

    private DataSource myDataSource;

    public DatabaseBean() {
        try {
            Context ctx = new InitialContext();
            ecwinsDataSource = (DataSource) ctx.lookup("jdbc/myDataSource");
        } catch (NamingException ex) {
            ex.printStackTrace();
        }
    }

    public Connection getConnection() throws ClassNotFoundException, SQLException, InstantiationException, IllegalAccessException {
        Connection connection = myDataSource.getConnection();
        System.out.println("Succesfully connected: " + connection);
        //Sample: Succesfully connected: com.sun.gjc.spi.jdbc40.ConnectionHolder40@7fb213a5
        return connection;
    }
}

This way the connection pool gets filled very fast; after a few navigation through 'db-related' views, the application stops with the following:

RAR5117 : Failed to obtain/create connection from connection pool [ mysql_ecwins_ecwinsPool ]. Reason : In-use connections equal max-pool-size and expired max-wait-time. Cannot allocate more connections. RAR5114 : Error allocating connection : [Error in allocating a connection. Cause: In-use connections equal max-pool-size and expired max-wait-time. Cannot allocate more connections.] java.sql.SQLException: Error in allocating a connection. Cause: In-use connections equal max-pool-size and expired max-wait-time. Cannot allocate more connections.
        at com.sun.gjc.spi.base.DataSource.getConnection(DataSource.java:115)
        at beans.DatabaseBean.getConnection(DatabaseBean.java:24)

I'm closing connections and other resources in every method. The application runs all OK with standalone connections.

What am I doing wrong? Any tips or advice would be appreciated.

EDIT

I looked into the way I am closing resources and it came out I wasn't very thorough. Now I'm closing them this way:

private void close(Connection connection, ResultSet rs, Statement stmt) {
    try {
        if (rs != null) {
            rs.close();
        }
    } catch (Exception ex) {
    } finally {
        try {
            if (stmt != null) {
                stmt.close();
            }
        } catch (Exception ex) {
        } finally {
            try {
                if (connection != null) {
                    connection.close();
                }
            } catch (Exception ex) {
            }
        }
    }
}

I don't get the error anymore! YAY, thanks!:)

+5  A: 

The exception indicates a typical case of application code which leaks DB connections. You need to ensure that you acquire and close all of them (Connection, Statement and ResultSet) in a try/finally block in the very same method block according the normal JDBC idiom:

public void create(Entity entity) throws SQLException {
    Connection connection = null;
    PreparedStatement statement = null;

    try { 
        connection = database.getConnection();
        statement = connection.prepareStatement(SQL_CREATE);
        statement.setSomeObject(1, entity.getSomeProperty());
        // ...
        statement.executeUpdate();
    } finally {
        if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
        if (connection != null) try { connection.close(); } catch (SQLException logOrIgnore) {}
    }
}

Yes, you still need to close connections yourself, even when using connection pooling. It's a common mistake among starters that they think that it will then automatically handle the close. This is not true. The connection pool namely returns a wrapped connection which does something like the following in the close():

public void close() throws SQLException {
    if (this.connection is still eligible for reuse) {
        do not close this.connection, but just return it to pool for reuse;
    } else {
        actually invoke this.connection.close();
    }
}

Not closing them would cause the connection not being released back to the pool for reuse and thus it will acquire a new one again and again until the DB runs out of connections which will cause your application to crash.

Also see this basic JDBC/DAO tutorial for more hints how to get started with JDBC (in webapplications) the proper way.

Update: you don't necessarly need to nest them that deep in their own finally blocks. The following is also just fine:

public static void closeQuietly(Conenction connection, Statement statement, ResultSet resultSet) {
    if (resultSet != null) try { resultSet.close(); } catch (SQLException logOrIgnore) {}
    if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
    if (connection != null) try { connection.close(); } catch (SQLException logOrIgnore) {}
}

which can be used as

} finally {
    closeQuietly(connection, statement, resultSet);
}
BalusC
Thank you for your thorough answer, it was really helpful! I edited my comment with the app-modifications.
wheelie
Thanks for the great info again!
wheelie
A: 

If you need JDBC connection pooling, why don't you rely on what's available already? AFAIK, JDBC connection pooling is considered more or less a standard feature in these java application servers, and IMO, you should not want to build this yourself if you're just interested in creating an application.

Here's a link that should get you started: http://weblogs.java.net/blog/2007/09/12/totd-9-using-jdbc-connection-pooljndi-name-glassfish-rails-application

What you probably should be doing is find out how to let your application grab a connection from the pool using jndi.

Roland Bouman
He's already doing that? He has already configured a connection pooled datasource in the appserver. He's only not closing resources properly as per the exception.
BalusC
oh crap, you're right. thanks BalusC.
Roland Bouman