Re: XAResource implementation

[Heikki Linnakangas <heikki(at)enterprisedb(dot)com>]

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;
      }