5
votes

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?

3

3 Answers

17
votes

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:

0
votes

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.