MySql too many connections

Question

I hate to bring up a question which is widely asked on the web, but I cant seem to solve it.

I started a project a while back and after a month of testing, I hit a "Too many connections" error. I looked into it, and "Solved" it by increasing the max_connections. This then worked.

Since then more and more people started to use it, and it hit again. When I am the only user on the site, i type "show processlist" and it comes up with about 50 connections which are still open (saying "Sleep" in the command). Now, I dont know enough to speculate why these are open, but in my code I tripple checked and every connection I open, I close.

ie.

public int getSiteIdFromName(String name, String company)throws DataAccessException,java.sql.SQLException{

Connection conn = this.getSession().connection();
Statement smt = conn.createStatement();
ResultSet rs=null;
String query="SELECT id FROM site WHERE name='"+name+"' and company_id='"+company+"'";

rs=smt.executeQuery(query);
rs.next();

int id=rs.getInt("id");

rs.close();
smt.close();
conn.close();
return id;
}

Every time I do something else on the site, another load of connections are opened and not closed. Is there something wrong with my code? and if not, what could be the problem?

Answer 1

If the code throws a DataAccessException or java.sql.SQLException the connection will not be closed resulting in many open sleeping connections ;) Make a try-finally-Block which will close the connection.

Connection conn = this.getSession().connection();
try {
  // all code
} finally {
  rs.close();
  smt.close();
  conn.close();
}

It's a trivial example but a little more complex because you have to check wich of these objects are really created and used.

Answer 2

With your approach, the connection will never be closed if any exception is been thrown before the conn.close() is called. You need to acquire it (and the statement and resultset) in a try block and close it in the finally block. Any code in finally will always be executed regardless of an exception is been thrown or not. With this you can ensure that the expensive resources will be closed.

Here's a rewrite:

public int getSiteIdFromName(String name, String company) throws DataAccessException, java.sql.SQLException {
    Connection conn = null;
    Statement smt = null;
    ResultSet rs = null;
    int id = 0;
    try {
        conn = this.getSession().connection();
        smt = conn.createStatement();
        String query = "SELECT id FROM site WHERE name='" + name + "' and company_id='" + company + "'";
        rs = smt.executeQuery(query);
        rs.next();
        id = rs.getInt("id");
    } finally {
        if (rs != null) try { rs.close(); } catch (SQLException logOrIgnore) {}
        if (smt != null) try { smt.close(); } catch (SQLException logOrIgnore) {}
        if (conn != null) try { conn.close(); } catch (SQLException logOrIgnore) {}
    }
    return id;
}

That said, this code is sensitive to SQL injection attacks. Use a PreparedStatement instead of Statement.

See also:

• Sun Exceptions tutorial: The Finally block
• Sun JDBC tutorial: introduction
• Sun JDBC tutorial: how to use PreparedStatement
• DAO tutorial: how to use basic JDBC code properly

Answer 3

One possible flow in which this code can leak connection is:

1. Stmt.executeQuery() results empty resultset
2. You do not check whether rs.next() returns true or false
3. rs.getInt("id") throws exception as there is not current row in resultset
4. conn.close() is skipped

Do the following:

1. Make rs.getInt() conditional on rs.next()
2. Close the connection in finally block and do all the data access within try block

Edit:

Also it's a good idea to log all exceptions somewhere so that you have a good starting point in your troubleshooting.