Re: a simple example of XA (not working)

Kris Jurka wrote:
>
>
> On Sun, 17 Jun 2007, Heikki Linnakangas wrote:
>
>> getConnection sets autocommit to false, so even though start set it to
>> true, it's reset to false in the call to getConnection. Attached patch
>> fixes that by explicitly setting autocommit to the right mode in
>> PGXAConnection.getConnection.
>>
>
> I don't think this fixes the problem completely if you have code that
> calls XAConnection.getConnection more than once:
>
> PGXADataSource xads = new PGXADataSource();
> XAConnection xaconn = xads.getXAConnection();
> XAResource xares = xaconn.getXAResource();
>
> xares.start(xid, XAResource.TMNOFLAGS);
>
> Connection conn1 = xaconn.getConnection();
> conn1.createStatement().executeUpdate(...);
>
> Connection conn2 = xaconn.getConnection();
>
> The second call to get connection will result in:
>
> setAutoCommit(true);
> setAutoCommit(false);
>
> on the real underlying connection, which will end up committing that
> part of the transaction.

You got me at first, but actually the second getConnection call will
*roll back* the first part of the transaction. See
AbstractJdbc23PooledConnection.getConnection. I don't think that's a
valid thing to do in the first place, though we could handle it more
gracefully. I added a note of that in the comment in
PGXAConnection.getConnection anyway.

I also added a comment block describing the three states a PGXAConection
object can be in.

There's still corner cases like the above, where the we really should
throw an error instead of getting confused. Another is that if you call
end(), and then use the connection for something else, that "else" will
become part of the transaction. And if you call
connection.commit/rollback/setAutoCommit, that will screw up the
not-yet-prepared transaction. It would be nice to catch and handle those
cases better, but that's more code.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
? build.local.properties
? foo.txt
? xa-PGObjectFactory-fix-2.patch
? xa-PGObjectFactory-fix.patch
? xa-autocommit-fix-2.patch
? xa-autocommit-fix.patch
? xa-endthenjoin-2.diff
? xa-endthenjoin.diff
? org/postgresql/xa/xa-autocommit-fix.patch
Index: org/postgresql/test/xa/XADataSourceTest.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/test/xa/XADataSourceTest.java,v
retrieving revision 1.8
diff -c -r1.8 XADataSourceTest.java
*** org/postgresql/test/xa/XADataSourceTest.java    1 Dec 2006 11:53:02 -0000    1.8
--- org/postgresql/test/xa/XADataSourceTest.java    25 Jun 2007 12:55:51 -0000
***************
*** 220,227 ****
--- 220,230 ----
      public void testAutoCommit() throws Exception {
          Xid xid = new CustomXid(6);

+         // When not in an XA transaction, autocommit should be true
+         // per normal JDBC rules.
          assertTrue(conn.getAutoCommit());

+         // When in an XA transaction, autocommit should be false
          xaRes.start(xid, XAResource.TMNOFLAGS);
          assertFalse(conn.getAutoCommit());
          xaRes.end(xid, XAResource.TMSUCCESS);
***************
*** 236,251 ****
--- 239,270 ----
          xaRes.commit(xid, false);
          assertTrue(conn.getAutoCommit());

+         // Check that autocommit is reset to true after a 1-phase rollback
          xaRes.start(xid, XAResource.TMNOFLAGS);
          xaRes.end(xid, XAResource.TMSUCCESS);
          xaRes.rollback(xid);
          assertTrue(conn.getAutoCommit());

+         // Check that autocommit is reset to true after a 2-phase rollback
          xaRes.start(xid, XAResource.TMNOFLAGS);
          xaRes.end(xid, XAResource.TMSUCCESS);
          xaRes.prepare(xid);
          xaRes.rollback(xid);
          assertTrue(conn.getAutoCommit());
+
+         // Check that autoCommit is set correctly after a getConnection-call
+         conn = xaconn.getConnection();
+         assertTrue(conn.getAutoCommit());
+
+         xaRes.start(xid, XAResource.TMNOFLAGS);
+
+         conn = xaconn.getConnection();
+         assertFalse(conn.getAutoCommit());
+
+         xaRes.end(xid, XAResource.TMSUCCESS);
+         xaRes.prepare(xid);
+         xaRes.rollback(xid);
+         assertTrue(conn.getAutoCommit());
      }

      public void testEndThenJoin() throws XAException {
Index: org/postgresql/xa/PGXAConnection.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/xa/PGXAConnection.java,v
retrieving revision 1.10
diff -c -r1.10 PGXAConnection.java
*** org/postgresql/xa/PGXAConnection.java    1 Dec 2006 11:53:02 -0000    1.10
--- org/postgresql/xa/PGXAConnection.java    25 Jun 2007 12:55:52 -0000
***************
*** 36,41 ****
--- 36,62 ----
      private final BaseConnection conn;
      private final Logger logger;

+     /*
+      * PGXAConnection-object can be in one of three states:
+      *
+      * IDLE
+      * Not associated with a XA-transaction. You can still call
+      * getConnection and use the connection outside XA. currentXid is null.
+      * autoCommit is true on a connection by getConnection, per normal JDBC
+      * rules, though the caller can change it to false and manage transactions
+      * itself using Connection.commit and rollback.
+      *
+      * ACTIVE
+      * start has been called, and we're associated with an XA transaction.
+      * currentXid is valid. autoCommit is false on a connection returned by
+      * getConnection, and should not be messed with by the caller or the XA
+      * transaction will be broken.
+      *
+      * ENDED
+      * end has been called, but the transaction has not yet been prepared.
+      * currentXid is still valid. You shouldn't use the connection for anything
+      * else than issuing a XAResource.commit or rollback.
+      */
      private Xid currentXid;

      private int state;
***************
*** 61,67 ****

      public Connection getConnection() throws SQLException
      {
!         return super.getConnection();
      }

      public XAResource getXAResource() {
--- 82,101 ----

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

      public XAResource getXAResource() {