Re: XAResource implementation - Mailing list pgsql-jdbc

joël Winteregg wrote:
> Many thanks for your answer and for the time you took to do some
> testing ! As you can see I put Ludovic Orban as Cc because he is the BTM
> developer and he seems to be interested to this issue too...
>
>> Hmm. I downloaded the BTM newUserDemo.zip and modified it to run with
>> Postgres, and to run two queries in same transaction. Works for me.
>>
>
> Ah Yes, you're right, your transaction just looks perfect !
>
>> Attached is the modified Test.java I used. To run:
>> 0. Download newUserDemo.zip from
>> http://docs.codehaus.org/display/BTM/NewUserGuide
>> 1. Put postgresql.jar in newUserDemo/lib
>> 2. Copy the attached Test.java to newUserDemo/src/jtatest
>> 3. Modify database/username/password in Test.java if necessary
>> 4. Run the CREATE TABLE from derby-create.sql
>>
>> Here's what I get in the Postgres log, with log_statements='all':
>>
>> LOG:  execute <unnamed>: SELECT gid FROM pg_prepared_xacts
>> LOG:  execute S_1: BEGIN
>> LOG:  execute <unnamed>: insert into messages(content) values ($1)
>> DETAIL:  parameters: $1 = 'hello, world!'
>> LOG:  execute S_2: COMMIT
>> LOG:  execute S_1: BEGIN
>> LOG:  execute <unnamed>: select content from messages
>> LOG:  execute <unnamed>: select content from messages
>> LOG:  execute S_2: COMMIT
>
> On my side, I tryed the following example:
> http://docs.codehaus.org/download/attachments/9240687/HibernateBTM.zip?version=2

Ok, I downloaded and installed that as well, and saw similar results.

It looks like there is indeed a bug in the Postgres driver. I believe it
was introduced by the recent changes to keep the connection in
autocommit mode when no XA-transaction is in progress.

It's this call sequence that borks it up:

xares.start()
conn = xares.getConnection()
// do stuff
conn.close();
conn = xares.getConnection()
// do more stuff
xares.end()
xares.commit()

The 2nd getConnection call inadvertently commits the transaction, and
opens another one.

Patch attached. I also added a test for this in the test suite.

Can you check that this fixes the issue for you, please? I can send you
a patched jar if you don't have build environment, let me know if you
need it.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
? xa-getConnection-fix.patch
Index: org/postgresql/ds/jdbc23/AbstractJdbc23PooledConnection.java
===================================================================
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/ds/jdbc23/AbstractJdbc23PooledConnection.java,v
retrieving revision 1.2
diff -c -r1.2 AbstractJdbc23PooledConnection.java
*** org/postgresql/ds/jdbc23/AbstractJdbc23PooledConnection.java    10 Sep 2007 08:38:15 -0000    1.2
--- org/postgresql/ds/jdbc23/AbstractJdbc23PooledConnection.java    13 Nov 2007 16:41:27 -0000
***************
*** 137,143 ****
                  }
                  con.clearWarnings();
              }
!             con.setAutoCommit(autoCommit);
          }
          catch (SQLException sqlException)
          {
--- 137,149 ----
                  }
                  con.clearWarnings();
              }
!             /*
!              * In XA-mode, autocommit is handled in PGXAConnection,
!              * because it depends on whether an XA-transaction is open
!              * or not
!              */
!             if (!isXA)
!                 con.setAutoCommit(autoCommit);
          }
          catch (SQLException sqlException)
          {
Index: org/postgresql/test/xa/XADataSourceTest.java
===================================================================
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/test/xa/XADataSourceTest.java,v
retrieving revision 1.9
diff -c -r1.9 XADataSourceTest.java
*** org/postgresql/test/xa/XADataSourceTest.java    6 Jul 2007 20:32:43 -0000    1.9
--- org/postgresql/test/xa/XADataSourceTest.java    13 Nov 2007 16:41:27 -0000
***************
*** 258,272 ****
--- 258,302 ----

          xaRes.start(xid, XAResource.TMNOFLAGS);

+         conn.createStatement().executeQuery("SELECT * FROM testxa1");
+
+         java.sql.Timestamp ts1 = getTransactionTimestamp(conn);
+
+         conn.close();
          conn = xaconn.getConnection();
          assertFalse(conn.getAutoCommit());

+         java.sql.Timestamp ts2 = getTransactionTimestamp(conn);
+
+         /* Check that we're still in the same transaction.
+          * close+getConnection() should not rollback the XA-transaction
+          * implicitly.
+          */
+         assertEquals(ts1, ts2);
+
          xaRes.end(xid, XAResource.TMSUCCESS);
          xaRes.prepare(xid);
          xaRes.rollback(xid);
          assertTrue(conn.getAutoCommit());
      }

+     /**
+      * Get transaction_timeout() from server.
+      *
+      * This can be used to check that transaction doesn't get committed/
+      * rolled back inadvertently, by calling this once before and after the
+      * suspected piece of code, and check that they match. It's a bit iffy,
+      * conceivably you might get the same timestamp anyway if the
+      * suspected piece of code runs fast enough, and/or the server clock
+      * is very coarse grained. But it'll do for testing purposes.
+      */
+     private static java.sql.Timestamp getTransactionTimestamp(Connection conn) throws SQLException
+     {
+         ResultSet rs = conn.createStatement().executeQuery("SELECT transaction_timestamp()");
+         rs.next();
+         return rs.getTimestamp(1);
+     }
+
      public void testEndThenJoin() throws XAException {
          Xid xid = new CustomXid(5);

Index: org/postgresql/xa/PGXAConnection.java
===================================================================
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/xa/PGXAConnection.java,v
retrieving revision 1.12
diff -c -r1.12 PGXAConnection.java
*** org/postgresql/xa/PGXAConnection.java    27 Jul 2007 10:15:39 -0000    1.12
--- org/postgresql/xa/PGXAConnection.java    13 Nov 2007 16:41:28 -0000
***************
*** 81,98 ****

      public Connection getConnection() throws SQLException
      {
          Connection conn = super.getConnection();

          // When we're outside an XA transaction, autocommit
          // is supposed to be true, per usual JDBC convention.
          // When an XA transaction is in progress, it should be
          // false.
!
!         // super.getConnection rolls back any previous transaction, and resets
!         // autocommit to true, so we have to set it to false before handing the
!         // connection to the caller, if an XA transaction is active.
!         if(state == STATE_ACTIVE)
!             conn.setAutoCommit(false);

          return conn;
      }
--- 81,97 ----

      public Connection getConnection() throws SQLException
      {
+         if (logger.logDebug())
+             debug("PGXAConnection.getConnection called");
+
          Connection conn = super.getConnection();

          // When we're outside an XA transaction, autocommit
          // is supposed to be true, per usual JDBC convention.
          // When an XA transaction is in progress, it should be
          // false.
!         if(state == STATE_IDLE)
!             conn.setAutoCommit(true);

          return conn;
      }