6

I'm using HikariCP 3.3.1 and PostgreSQL. But I've a problem with closing my connections, in Hikari config I set maximum pool size to 15 and minimum idle connection to 5, but after a few minutes of work with database I've found out connections don't closes, they stack more and more (almost 100 Idle connections right now). enter image description here

My Connector class:

Connector.java

public class Connector implements IConnector {
private static HikariConfig config = new HikariConfig();
private static HikariDataSource ds;

static {
    config.setDriverClassName(org.postgresql.Driver.class.getName());
    config.setJdbcUrl("jdbc:postgresql://localhost:5432/vskDB");
    config.setUsername("postgres");
    config.setPassword("root");
    config.setMinimumIdle(5);
    config.setMaximumPoolSize(15);
    config.setConnectionTimeout(20000);
    config.setIdleTimeout(300000);
    ds = new HikariDataSource(config);
}

public Connection getConnection() {
    log.info("getConnection() invoked");
    try {
        return ds.getConnection();
    } catch (SQLException e) {
        log.error("Can't get connection from DataSource.");
        log.error(e.getMessage());
        System.out.println(e.getMessage());
    }
    return null;
}

Connector() {
}
}

And here's my DAO class (simplified): UserDAO.java

public class UserDatabaseDAO implements UserDAO {
    private Connector connector = new Connector();
    private Connection dbConnection;

    @Override
    public void removeUser(Long id) {
        try {
            dbConnection = connector.getConnection();
            if (dbConnection == null)
                throw new ConnectException();

            PreparedStatement preparedStatement = dbConnection.prepareStatement("DELETE FROM users WHERE user_id = ?");
            preparedStatement.setLong(1, id);
            preparedStatement.execute();

        } catch (SQLException | ConnectException e) {
            log.error("Can't remove user from database");
            log.error(e.getMessage());
            System.out.print(e.getMessage());
        } finally {
            try {
                dbConnection.close();
            } catch (SQLException e) {
                log.error("Can't close connection");
                log.error(e.getMessage());
                System.out.print(e.getMessage());
            }
        }
    }
}

Here I've found an issue with some facts about Hikari:
You must call close() on the connection instance that HikariCP gives you

Maybe my dbConnection.close() wont work because it's just a copy of Connection which Hikari gives me in getConnection() method.

Nikita Kalugin
  • 682
  • 2
  • 14
  • 37
  • It is a bit unclear what you are asking. The whole point of a connection pool like HikariCP is to keep connections open for reuse. So that connections remain open is not unexpected, it is the whole point. When you call `close()` on a connection from a HikariCP datasource, it is returned to the connection pool, the connection is not physically closed. – Mark Rotteveel May 26 '19 at 16:06
  • I notice that all your connections have a different PID, that suggests that you are starting multiple processes that never end. – Mark Rotteveel May 26 '19 at 16:12
  • 1
    @MarkRotteveel I've used try-with-resource from Java 7 and now here's only 15 Idle connection, as I set in Max pool size. But they don't closes after idleTimeout. I've set maxLifetime and idleTimeout to default. Can it be because of auto-commit or something else? By the way, Connection's "Backend start" change sometimes. – Nikita Kalugin May 26 '19 at 18:52

1 Answers1

4

You forgot to close also PreparedStatement

try {
       if (preparedStatement != null) {
            preparedStatement.close();
       }
       if (dbConnection != null) {
            dbConnection.close();
       }

Releases this Statement object's database and JDBC resources immediately instead of waiting for this to happen when it is automatically closed. It is generally good practice to release resources as soon as you are finished with them to avoid tying up database resources.

NateS
  • 5,751
  • 4
  • 49
  • 59
Ori Marko
  • 56,308
  • 23
  • 131
  • 233
  • Hmm, maybe you're right. Can I use try-with-multiple resource? In my case with connection.getConnection() and PreparedStatement declaration to close them both? – Nikita Kalugin May 27 '19 at 05:01
  • @EDWIN it's possible, you can *try* by putting methods which return Connection PreparedStatement inside try-with-resources block – Ori Marko May 27 '19 at 05:05
  • Is this ok to do this: `try (Connection dbConnection = connector.getConnection(); PreparedStatement preparedStatement = dbConnection.prepareStatement("SELECT * FROM users WHERE user_id = ?") ) { ` What if `connector.getConnection()` return null, as I think it will be NullPointerException in this place `dbConnection.prepareStatement` – Nikita Kalugin May 27 '19 at 05:06
  • 1
    @EDWIN so let the method throw ConnectException instead of null. see also https://stackoverflow.com/questions/33703905/is-there-a-cleaner-way-to-use-try-with-resource-and-preparedstatement – Ori Marko May 27 '19 at 05:13
  • You just saved my life. Thank you so much. – Nikita Kalugin May 27 '19 at 05:25
  • One last question, should I close ResultSet? – Nikita Kalugin May 27 '19 at 05:55
  • 1
    @EDWIN yes if you are using, but I didn't see in your question – Ori Marko May 27 '19 at 06:24
  • A connection close should close a statement, and if you do close both explicitly, then you should close the statement **before** the connection. – Mark Rotteveel May 27 '19 at 07:39