12

After some time of running I'm getting this error when I stress test my servlet with at least 20 browser tabs simultaneously accessing the servlet:

java.sql.SQLException: [tomcat-http--10] Timeout: Pool empty. Unable to fetch a connection in 10 seconds, none available[size:200; busy:200; idle:0; lastwait:10000].

Here is the XML config for this:

<Resource name="jdbc/MyAppHrd"
          auth="Container"
          type="javax.sql.DataSource"
          factory="org.apache.tomcat.jdbc.pool.DataSourceFactory"
          testWhileIdle="true"
          testOnBorrow="true"
          testOnReturn="false"
          validationQuery="SELECT 1"
          validationInterval="30000"
          timeBetweenEvictionRunsMillis="30000"
          maxActive="200"
          minIdle="10"
          maxWait="10000"
          initialSize="200"
          removeAbandonedTimeout="120"
          removeAbandoned="true"
          logAbandoned="false"
          minEvictableIdleTimeMillis="30000"
          jmxEnabled="true"
          jdbcInterceptors="org.apache.tomcat.jdbc.pool.interceptor.ConnectionState;
            org.apache.tomcat.jdbc.pool.interceptor.StatementFinalizer"
          username="sa"
          password="password"
          driverClassName="net.sourceforge.jtds.jdbc.Driver"
          url="jdbc:jtds:sqlserver://192.168.114.130/MyApp"/>

What could be the problem?

Update: Java Code:

public class MyServlet extends HttpServlet {

    private static final long serialVersionUID = 1L;
    private static final Log LOGGER = LogFactory.getLog(MyServlet.class);

   private void doRequest(HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {

        CallableStatement stmt = null;
        ResultSet rs = null;

        Connection conn = null;
        try {

            conn = getConnection();

            stmt = conn.prepareCall("{call sp_SomeSPP(?)}");
            stmt.setLong(1, getId());

            rs = stmt.executeQuery();

            // set mime type
            while (rs.next()) {
                if (rs.getInt(1)==someValue()) {
                    doStuff();
                    break;
                }
            }
            stmt = conn.prepareCall("{call sp_SomeSP(?)}");
            stmt.setLong(1, getId());

            rs = stmt.executeQuery();
            if (rs.next()) {
                // do stuff
            }

            RequestDispatcher rd = getServletContext().getRequestDispatcher("/SomeJSP.jsp");
            rd.forward(request, response);
            return;
        } catch (NamingException e) {
            LOGGER.error("Database connection lookup failed", e);
        } catch (SQLException e) {
            LOGGER.error("Query failed", e);
        } catch (IllegalStateException e) {
            LOGGER.error("View failed", e);
        } finally {
            try {
                if (rs!=null && !rs.isClosed()) {
                    rs.close(); 
                }
            } catch (NullPointerException e) {
                LOGGER.error("Result set closing failed", e);
            } catch (SQLException e) {
                LOGGER.error("Result set closing failed", e);
            }
            try {
                if (stmt!=null) stmt.close();
            } catch (NullPointerException e) {
                LOGGER.error("Statement closing failed", e);
            } catch (SQLException e) {
                LOGGER.error("Statement closing failed", e);
            }
            try {
                if (conn != null){
                    conn.close();
                    conn = null;
                }
            } catch (NullPointerException e) {
                LOGGER.error("Database connection closing failed", e);
            } catch (SQLException e) {
                LOGGER.error("Database connection closing failed", e);
            }
        }

   }

    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        doRequest(request, response);
    }

    @Override
    protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        doRequest(request, response);
    }

    protected static Connection getConnection() throws NamingException, SQLException {
        InitialContext cxt = new InitialContext();
        String jndiName = "java:/comp/env/jdbc/MyDBHrd";
        ConnectionPoolDataSource dataSource = (ConnectionPoolDataSource) cxt.lookup(jndiName);
        PooledConnection pooledConnection = dataSource.getPooledConnection();
        Connection conn = pooledConnection.getConnection();
        return conn; // Obtain connection from pool
    }   
quarks
  • 33,478
  • 73
  • 290
  • 513
  • 4
    You probably got unclosed connections – Aviram Segal Dec 20 '12 at 16:25
  • I'm sure i have the try-finally block where the db stuff gets done in the try and in finally, the I call conn.close(), and I did put a breakpoint to test whether each refresh will call close() – quarks Dec 20 '12 at 16:31
  • So maybe the pool is just not big enough for the load you put on it ? – Aviram Segal Dec 20 '12 at 16:33
  • 1
    Can you rather provide some code where you're actually using the connection? The declaration you mention just states that you have a maximum size of 200 connections in the pool. I'd expect this to be enough to serve 20 browser tabs, otherwise you'll have to worry about your coding style (which is what I suspect). As Aviram Segal mentions, it's most likely a bunch of unclosed connections. Or you have loooong-running processes in there. – Olaf Kock Dec 20 '12 at 16:36
  • @OlafKock I've updated the post with the associated java code. The doRequest() is pretty straightforward, it gets data from Stored procedure that returns data in a snap. Not really doing any intensive computation here. – quarks Dec 20 '12 at 16:42

6 Answers6

5

I suggest you change your getConnection method to the following you might actually be removing the Pooling support by going directly to via the javax.sql.PooledConnection interface

        InitialContext cxt = new InitialContext();
        String jndiName = "java:/comp/env/jdbc/MyDBHrd";
        DataSource dataSource = (DataSource) cxt.lookup(jndiName);
        return dataSource.getConnection();

Also use something like DBUtils#closeQuietly to clean up your connections

Update: You are removing the Pooling support from the Connection. If you run the following and look at the output you will see the connection retrieved directly from the DataSource is a ProxyConnection wrapping a PooledConnection.

public static void main(String[] args) throws Exception {

    Properties properties = new Properties();
    properties.put("username", "sa");
    properties.put("password", "password");
    properties.put("driverClassName", "net.sourceforge.jtds.jdbc.Driver");
    properties.put("url", "jdbc:jtds:sqlserver://192.168.114.130/MyApp");       

    DataSourceFactory dsFactory = new DataSourceFactory();      
    DataSource ds = dsFactory.createDataSource(properties);     
    ConnectionPoolDataSource cpds = (ConnectionPoolDataSource) ds;
    PooledConnection pooledConnection = cpds.getPooledConnection();

    System.out.println("Pooled Connection - [" + ds.getConnection() + "]"); // Close will return to the Pool
    System.out.println("Internal Connection - [" + pooledConnection.getConnection() + "]"); // Close will just close the connection and not return to pool

}
bstick12
  • 1,699
  • 12
  • 18
  • You mean, not use: Connection conn = pooledConnection.getConnection(); ? – quarks Dec 20 '12 at 18:06
  • I'm not getting what you mean? – quarks Dec 20 '12 at 18:09
  • If I use your code above, I get this error: java.sql.SQLException: Connection has already been closed. – quarks Dec 20 '12 at 18:28
  • if you just redeployed your changed code to the running tomcat, you might have closed the connections before with your old code. Restart tomcat and try again, this will start with a new pool of connections – Olaf Kock Dec 20 '12 at 19:32
  • I've mocked up a quick test locally and you are removing the pooling support from the connection by going via the ConnectionPoolDataSource#getPolledConnection#getConnection. The SQLException could be due to you not restarting the pool correctly with Tomcat. – bstick12 Dec 21 '12 at 09:27
  • @xybrek You should never access a `PooledConnection` directly yourself: that is for internal use by a connection pool. See also my answer here: http://stackoverflow.com/a/12651163/466862 – Mark Rotteveel Dec 21 '12 at 09:48
  • I'm getting SQLException: Invalid state, the Connection object is closed. when accessed simultaneously from a web browser (chrome) with your code – quarks Dec 23 '12 at 17:55
4

Probably, you are holding connection for too long.

Make sure that you do not open DB connection when you start processing request and then release it when you finally committed the response.

Typical mistake is:

    @Override
    protected void doGet (
            final HttpServletRequest request,
            final HttpServletResponse response
        ) throws
            ServletException,
            IOException
    {
        Connection conn = myGetConnection( );

        try
        {
            ...
            // some request handling


        }
        finally
        {
            conn.close( )
        }
    }

In this code, database connection lifetime is totally at the mercy of the client connected to your server.

Better pattern would be

    @Override
    protected void doGet (
            final HttpServletRequest request,
            final HttpServletResponse response
        ) throws
            ServletException,
            IOException
    {
        // some request preprocessing
        MyProcessedRequest parsedInputFromRequest =
            getInputFromRequest( request );

        final MyModel model;
        {
           // Model generation
           Connection conn = myGetConnection( );

           try
           {
              model = new MyModel( conn, parsedInputFromRequest );
           }
           finally
           {
              conn.close( );
           }
        }


        generateResponse( response, model );         
    }

Note, that if the bottleneck is in model generation, you still going to run out of connections, but this is now a problem for DBA, that relates to better data management/indexing on the database side.

Alexander Pogrebnyak
  • 44,836
  • 10
  • 105
  • 121
1

Check your jdbc connections are closed after completion of process. It may caused by unclosed connections.

Kanagaraj M
  • 956
  • 8
  • 18
0

Close your connection before the following.

RequestDispatcher rd = getServletContext().getRequestDispatcher("/SomeJSP.jsp");
        rd.forward(request, response);
        return;

Also remove return if it is not required.

someone
  • 6,577
  • 7
  • 37
  • 60
0

The code you currently provide looks long/complex, but fine.

However, I guess your "doStuff" method might be a candidate for leaking more connections

Olaf Kock
  • 46,930
  • 8
  • 59
  • 90
  • the doStuff() function is actually just a assignment line of code. The actual line is just an assignment: lastUpdated = rs.getTimestamp("LastUpdated"); – quarks Dec 20 '12 at 17:24
  • so - you obviously access the database in there... care to show the code? – Olaf Kock Dec 20 '12 at 17:32
  • the SQL stored procedure is a T-SQL, with a basic code to call MAX() function – quarks Dec 20 '12 at 18:24
0

First, you are not closing your Statement and ResultSet objects within the body of your method.

They should be cleaned-up when you call close on the the Connection (according to the JDBC spec), but in a pooled setting, they might not actually get cleaned up.

Second, you are unwrapping the pooled connection and returning the underlying connection which will break everything.

So, modify your code to be like this:

 try {
        conn = getConnection();

        stmt = conn.prepareCall("{call sp_SomeSPP(?)}");
        stmt.setLong(1, getId());

        rs = stmt.executeQuery();

        // set mime type
        while (rs.next()) {
            if (rs.getInt(1)==someValue()) {
                doStuff();
                break;
            }
        }

        // ADD THESE LINES
        rs.close(); rs = null;
        stmt.close(); stmt = null;

        stmt = conn.prepareCall("{call sp_SomeSP(?)}");
        stmt.setLong(1, getId());

        rs = stmt.executeQuery();
        if (rs.next()) {
            // do stuff
        }
}

....

protected static Connection getConnection() throws NamingException, SQLException {
    InitialContext cxt = new InitialContext();
    String jndiName = "java:/comp/env/jdbc/MyDBHrd";
    DataSource dataSource = (DataSource) cxt.lookup(jndiName);
    return dataSource.getPooledConnection();
}   

And, as others have said, you definitely want to clean-up your resources before you do things like forwarding to another page. Otherwise, you keep the connection far longer than necessary. It's a critical resource: treat it like one.

Christopher Schultz
  • 20,221
  • 9
  • 60
  • 77
  • I modified my code to fit your code, here's what I get: SQLException: Invalid state, the Connection object is closed. when accessed simultaneously from a web browser (chrome) – quarks Dec 20 '12 at 19:31
  • Do you get a stack trace with the new exception? If so, post it as an edit to your question. If you call `close()` twice, you'll get this error. – Christopher Schultz Dec 25 '12 at 21:04