5

I have an app that I'm connecting to a MySQL database. It loses connection in the middle of the night and then spouts about null connections and JDBC hasn't received messages in X seconds.

I call getConnection() before I do anything that requires communication with the SQL server.

This is my getConnection() method:

private Connection getConnection() {
    try {
        if (connection != null) {
            if (connection.isClosed() || !connection.isValid(10000)) {
                this.initializeRamsesConnection();
            }
        } else {
            this.initializeRamsesConnection();
        }
    } catch (Exception e) {
        debug("Connection failed: " + e);
    }
    return connection;
}

In the initializeRamsesConnection() method I put the password and so on information into a string and then I create the connection in the standard JDBC way.

Then I call this method:

private Connection getConnectionFromConnectionString() {
    Connection con = null;
    String driver = "com.mysql.jdbc.Driver";
    try {
        Class.forName(driver);//jdbc sorcery
        //if there is no connection string
        if (getConnectionString() == null) {
            HMIDatabaseAdapter.debug("No connection string");
        }
        //makes a string out of the values of db/host
        String str = getConnectionString();
        //if there is no driver
        if (driver == null) {
            debug("" + ": " + "No driver");
        }
        //Tries to make a connection from the connection string, username, and the password.
        con = DriverManager.getConnection(str, username, password);
        //if for some reason the connection is null
        if (con == null) {
            HMIDatabaseAdapter.debug("CONNECTION IS NULL, WHAT?");
        }
    } catch (Exception ex) {
        HMIDatabaseAdapter.debug("getConnection() " + ex);
    }
    return con;
}

What can I change in either of these methods to accommodate losing connection?

Regent
  • 5,142
  • 3
  • 21
  • 35
davidahines
  • 3,976
  • 16
  • 53
  • 87

3 Answers3

17

This is not the correct way of retrieving a connection. You're retrieving the connection and assigning it as an instance (or worse, static) variable of the class. Basically, you're keeping the connection open forever and reusing a single connection for all queries. This may end up in a disaster if the queries are executed by different threads. Also, when it's been kept open for too long, the DB will reclaim it because it assumes that it's dead/leaked.

You should acquire and close the connection in the shortest possible scope. I.e. in the very same try block as where you're executing the query. Something like this:

public Entity find(Long id) throws SQLException {
    Entity entity = null;

    try (
        Connection connection = dataSource.getConnection(); // This should return a NEW connection!
        PreparedStatement statement = connection.prepareStatement(SQL_FIND);
    ) {
        statement.setLong(1, id);

        try (ResultSet resultSet = preparedStatement.executeQuery()) {
            if (resultSet.next()) {
                entity = new Entity(
                    resultSet.getLong("id"),
                    resultSet.getString("name"),
                    resultSet.getInt("value")
                );
            }
        }
    }       

    return entity;
}

If you worry about connecting performance and want to reuse connections, then you should be using a connection pool. You could homegrow one, but I strongly discourage this as you seem to be pretty new to the stuff. Just use an existing connection pool like BoneCP, C3P0 or DBCP. Note that you should not change the JDBC idiom as shown in the above example. You still need to acquire and close the connection in the shortest possible scope. The connection pool will by itself worry about actually reusing, testing and/or closing the connection.

See also:

Community
  • 1
  • 1
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • 1
    The queries aren't executed by different threads. Ever. It is a single threaded application. If you close the connection in every try block then it quickly becomes expensive. – davidahines Dec 01 '11 at 18:39
  • That's exactly why you want a connection pool. Instead of homegrowing it -as you attempted to do with this quesiton- just use an existing connection pool library. – BalusC Dec 01 '11 at 18:41
  • I'm not looking to rearchitect something that already works and only loses connection every few days, I know about BoneCP, I'm just looking to get it to reconnect when it loses connection. I don't have the time to rewrite it or else I would. – davidahines Dec 01 '11 at 18:41
  • 2
    If your JDBC code is properly written, it's just a matter of changing the global `getConnection()` method to replace `DriverManager#getConnection()` by `DataSource#getConnection()`. See also the "See also" links for some concrete examples. – BalusC Dec 01 '11 at 18:42
  • Ahh. In that case. I retract my prior statements. This is useful, thank you BalusC. – davidahines Dec 01 '11 at 18:49
  • 2
    You're welcome. Don't forget to close the connection after every query so that it get returned to the pool :) – BalusC Dec 01 '11 at 18:53
  • Ahh! So it doesn't really close it then? That sounds interesting. Do you use a different connection object with BONECP? – davidahines Dec 01 '11 at 19:10
  • 1
    Indeed, see also my answer on the last "See also" link. – BalusC Dec 01 '11 at 19:11
  • Will do. Likely I'll have to do it in my offtime if it isn't like a 20-30 minutes fix as I'm super short on time here. – davidahines Dec 01 '11 at 20:33
  • I strongly discourage closing the connection each time you execute a prepared statement. You are generating very much cost for the network + db. Try to reuse the prepared statement as long as it has an open connection. If you want the full answer, just request it here. – MartinL Nov 16 '12 at 14:38
  • @MartinL: apparently you missed the paragraph after the code block which says something about performance and connection pools. I suggest to read it as well and reconsider the ridiculous downvote+comment. As to reusing prepared statements, that's already internally done and there's always the `addBatch()`/`executeBatch()` for the case you want to execute multiple of them in *one transaction*. – BalusC Nov 16 '12 at 14:39
  • @MartinL: see also http://stackoverflow.com/a/3273378/157882 and http://stackoverflow.com/a/2467152/157882 – BalusC Nov 16 '12 at 14:47
0

Where in your code are the errors on losing connection coming from? This would probably be the best place to start.

Off the top of my head (and I may be wrong), JDBC connections will only close on an actual fatal error, so you won't know they've failed until you try to do something.

What I've done in the past is to invalidate the connection at the point of failure and retry periodically.

Tom Elliott
  • 1,908
  • 1
  • 19
  • 39
0

Maybe this is what you are looking for: http://dev.mysql.com/doc/refman/5.0/en/auto-reconnect.html

For java see autoReconnect: http://dev.mysql.com/doc/refman/5.0/en/connector-j-reference-configuration-properties.html

Joop Eggen
  • 107,315
  • 7
  • 83
  • 138