Propagating autocommit state to JDBC connection during cleanup phase

[thofman]

Hello,

I'm looking at Bug 1195079 – "org.postgresql.util.PSQLException: Cannot change transaction isolation" at calling Connection#setTransact… and would like to discuss a solution.

Situation:

1) We have data source with following validation setting:

<validation>

  <check-valid-connection-sql>select 1</check-valid-connection-sql>

  <validate-on-match>true</validate-on-match>

</validation>

2) Application obtains a connection via ds.getConnection(). (Obtained connection is a WrappedConnection.)

3) Application sets autocommit = false, changes transaction isolation, does stuff, commits, sets autocommit = true, terminates. This is OK during first request, but during second request, exception is thrown when changing transaction isolation:

Error resetting transaction isolation : org.postgresql.util.PSQLException: Cannot change transaction isolation level in the middle of a transaction.

  at org.postgresql.jdbc2.AbstractJdbc2Connection.setTransactionIsolation(AbstractJdbc2Connection.java:929)...

The problem is that wrapped ("real") jdbc connection is left in autocommit = false, even after wrapper connection cleanup. So when it's obtained for second request, validation with "select 1" actually starts new transaction, and setTransactionIsolation() fails because of that.

I think this could be solved by propagating autocommit state into wrapped jdbc connection during connection cleanup (BaseWrapperManagedConnection#cleanup()) like that [1], which is not done currently.

[1] [BZ-1195079] Cannot change transaction isolation · 1188b54 · TomasHofman/ironjacamar · GitHub

[jesper.pedersen]

No, you can't change the underlying state in cleanup() - see comment in destroy().

Any test cases + patches should be submitted against the 1.2 branch, and not the 1.0 branch, as it is for approved backports only.

I suggest you look for inspiration in pgjdbc/ConnectionTest.java at master · pgjdbc/pgjdbc · GitHub for your test case that identifies this issue. There are a number of JDBC related test cases in IronJacamar already to look at. Also include the full configuration of your datasource, the JDBC driver version, and server version in your next post.

[thofman]

PostgreSQL JDBC driver version is 9.3-1103-jdbc41

PostgreSQL server version is 9.3.6

Full data source configuration:

<datasource jndi-name="java:/H2DS" pool-name="H2DS">
  <connection-url>jdbc:postgresql://localhost:5432/jboss</connection-url>
  <driver-class>org.postgresql.Driver</driver-class>
  <security>
  <user-name>postgres</user-name>
  <password>postgres</password>
  </security>
   <pool>
  <min-pool-size>1</min-pool-size>
  <max-pool-size>1</max-pool-size>
  <prefill>true</prefill>
  <use-strict-min>true</use-strict-min>
  </pool>
  <validation>
  <check-valid-connection-sql>select 1</check-valid-connection-sql>
  <validate-on-match>true</validate-on-match>
  </validation>
  </datasource>

The problem only happens with PostgreSQL, H2 for instance allows to change isolation level in the middle of the transaction, so it's not problem there.

I implemented a test, with mocked JDBC driver that simulates PostgreSQL behaviour: [BZ-1195079] Cannot change transaction isolation · 8973720 · TomasHofman/ironjacamar · GitHub

So, if we cannot reset autocommit state during cleanup, can we reset it when the connection is being obtained from a pool? This would do the job: [BZ-1195079] Cannot change transaction isolation · 1a008b7 · ironjacamar/ironjacamar · GitHub

[jesper.pedersen]

You are jumping ahead, start by doing a test case for the PostgreSQL driver - f.ex. by adding to their existing test case. Then test using your installation and latest versions.

If you are saying that other databases doesn't complain (don't count H2) then it sounds more like a JDBC driver problem -- missing reset of state.

[thofman]

> "You are jumping ahead, start by doing a test case for the PostgreSQL driver - f.ex. by adding to their existing test case."

Of course I was running that test of mine also with real PostgreSQL driver, with the ds configuration I posted above, just didn't commit it. However, I don't think I can test this inside *their* (pgjdbc) test suite, since I'm testing IronJacamar behaviour...

> "sounds more like a JDBC driver problem -- missing reset of state"

My point is when is that reset supposed to happen? As I see it, IronJacamar never tells the driver to set autocommit back to true between returning the connection to a pool and retrieving it from the pool again. That doesn't matter for the application code, since application is working with WrappedConnection, which is checking and fixing autocommit during all operations like prepareStatement() and others, but ValidConnectionChecker operates with physical JDBC connection directly and therefore can open undesired transaction if autocommit is left disabled in physical connection.

I was testing MariaDB, Oracle and Firebird databases. From those, only Oracle doesn't allow changing transaction isolation during transaction, though it's not as strict as PostgreSQL:

- Oracle only throws an exception during conn.setTransactionIsolation() if there was already some update or insert statement in the transaction, it's OK with only select statements executed.

- PostgreSQL throws an exception even if there were only select statements in the transaction.

- MariaDB and Firebird doesn't complain about changing transaction isolation during transaction at all.

Also note that documentation of Connection.setTransactionIsolation() says: "If this method is called during a transaction, the result is implementation-defined."

So I can reproduce the problem with PostgreSQL and Oracle, although with Oracle it requires a little weird setting of check-valid-connection-sql to an update statement. That of course is unlikely to be used in practice, but at least it demonstrates that autocommit of an jdbc driver is left disabled.

I committed separate tests for PostgreSQL and Oracle alongside the original one:

ironjacamar/PostgresTransactionIsolationTestCase.java at 8ecab192c6c479e903cb5d7be0bc579b182853b1 · TomasHofman/ironjaca…

ironjacamar/OracleTransactionIsolationTestCase.java at 8ecab192c6c479e903cb5d7be0bc579b182853b1 · TomasHofman/ironjacama…

Tested JDBC driver version were:

Oracle 11.2.0.4

PostgreSQL 9.3-1103-jdbc41, 9.4-1201.jdbc41