brettwooldridge / HikariCP

光 HikariCP・A solid, high-performance, JDBC connection pool at last.
Apache License 2.0
19.91k stars 2.92k forks source link

Connection with active transaction returned to the pool when autoCommit=false, default schema defined and using pgjdbc #2144

Open andeb opened 10 months ago

andeb commented 10 months ago

When HikariCP returns a connection to the pool, it calls resetConnectionState method, that will call setSchema if HikariCP is configured with a default schema and the schema was changed during the usage of the connection (more common when using multi-tenancy per schema pattern). This call from Hikari to setSchema will make the pgjdbc driver send a BEGIN before executing the SET SESSION search_path, leaving the pool with an active transaction in it.

Should Hikari be sending a commit after changing the schema autoCommit is false?

More information on https://github.com/pgjdbc/pgjdbc/issues/3005

To Reproduce Run the application below:

package org.example;

import com.zaxxer.hikari.HikariDataSource;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.concurrent.TimeUnit;
import java.util.logging.ConsoleHandler;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.logging.SimpleFormatter;
import org.postgresql.Driver;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.lifecycle.Startables;

public class Main {

  public static void main(String[] args) throws Exception {
    ConsoleHandler consoleHandler = new ConsoleHandler();
    consoleHandler.setFormatter(new SimpleFormatter());
    consoleHandler.setLevel(Level.ALL);

    Logger driverLogger = Logger.getLogger("org.postgresql.core.v3.QueryExecutorImpl");
    driverLogger.setLevel(Level.ALL);
    driverLogger.addHandler(consoleHandler);

    Logger logger = Logger.getLogger("main");
    logger.setLevel(Level.ALL);
    logger.addHandler(consoleHandler);

    try (var container = new PostgreSQLContainer("postgres:15.0-alpine")) {
      Startables.deepStart(container).get();
      TimeUnit.SECONDS.sleep(3);

      try (HikariDataSource ds = new HikariDataSource()) {
        ds.setJdbcUrl(container.getJdbcUrl());
        ds.setUsername(container.getUsername());
        ds.setPassword(container.getPassword());
        ds.setAutoCommit(false);
        ds.setDriverClassName(Driver.class.getName());
        ds.setSchema("public");

        Connection connection = ds.getConnection();

        logger.info("Started SetSchema");
        // Set to a different schema to dirty bits on Hikari
        connection.setSchema("another_schema");
        connection.commit();

        logger.info("Close Connection");
        // At this point Hikari will set the schema back to public and the driver will start a transaction
        // FINEST:  FE=> Parse(stmt=null,query="BEGIN",oids={})
        // FINEST:  FE=> Parse(stmt=null,query="SET SESSION search_path TO 'public'",oids={})
        connection.close();
      }
    } catch (SQLException e) {
      throw new RuntimeException(e);
    }
  }
}
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>

  <groupId>org.example</groupId>
  <artifactId>pgjdbc-setschema</artifactId>
  <version>1.0-SNAPSHOT</version>

  <properties>
    <maven.compiler.source>21</maven.compiler.source>
    <maven.compiler.target>21</maven.compiler.target>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>

  <dependencies>
    <dependency>
      <groupId>org.postgresql</groupId>
      <artifactId>postgresql</artifactId>
      <version>42.7.0</version>
    </dependency>

    <dependency>
      <groupId>com.zaxxer</groupId>
      <artifactId>HikariCP</artifactId>
      <version>5.1.0</version>
    </dependency>

    <dependency>
      <groupId>org.testcontainers</groupId>
      <artifactId>postgresql</artifactId>
      <version>1.19.2</version>
    </dependency>
  </dependencies>
</project>
quaff commented 10 months ago

I think changing schema/catalog shouldn't be part of transaction, it should be fixed at PG side.

kevinchan-rl commented 3 weeks ago

I am also noticing this behavior with autoCommit=false and a default schema when a connection is recreated due to maxLifetime being reached, after which point the connection transaction state remains OPEN and we would expect it to be IDLE because when we get the connection and try to set isolation level it now blows up with a "Cannot change transaction isolation level in the middle of a transaction` PSQLException

kevinchan-rl commented 3 weeks ago

attached is the postgres logs for the initial connection creation and subsequently after maxLifetime is reached and you can see hikari leaves the transaction open when autoCommit=false auto_commit_false.txt auto_commit_true.txt

kevinchan-rl commented 3 weeks ago

in case it's not a duplicate, I created https://github.com/brettwooldridge/HikariCP/issues/2235