Re: Possible oversight in org.postgresql.xa.PGXAConnection.commitPrepared(Xid xid) - Mailing list pgsql-jdbc

I had to adjust the patch so that the new variable xaerrcode was passed into the PGXAException constructor (on line 522
ofthe patched code) rather than XAException.XAER_RMERR.  Once that was done recovery worked just as expected.  Thanks
foryour work on this! 


Justin

----- Original Message -----
From: "Heikki Linnakangas" <heikki.linnakangas@enterprisedb.com>
To: "Justin Bertram" <jbertram@redhat.com>
Cc: pgsql-jdbc@postgresql.org
Sent: Wednesday, June 29, 2011 4:26:51 AM
Subject: Re: [JDBC] Possible oversight in org.postgresql.xa.PGXAConnection.commitPrepared(Xid xid)

On 29.06.2011 01:52, Justin Bertram wrote:
> I'm bringing this back up because, while the XAER_RMERR works in most cases it fails in at least one.
>
> Consider the scenario where the database is shutdown administratively during
org.postgresql.xa.PGXAConnection.commitPrepared(Xidxid).  The driver will throw an XAException with an errorCode of
XAER_RMERRback to the transaction manager.  However, the pg_prepared_xacts table will still contain a row for the
transaction.
>
> The method org.postgresql.xa.PGXAConnection.commitPrepared(Xid xid) is invoked by the transaction manager as part of
itscall to javax.transaction.xa.XAResource.commit(..) [1].  This is the JTA mapping of the xa_commit() function from
theXA specification [2].  According to this document, a return of XAER_RMERR means: 
>
>    An error occurred in committing the work performed on behalf of the transaction
>    branch and the branch’s work has been rolled back. Note that returning this error
>    signals a catastrophic event to a transaction manager since other resource
>    managers may successfully commit their work on behalf of this branch. This error
>    should be returned only when a resource manager concludes that it can never
>    commit the branch and that it cannot hold the branch’s resources in a prepared
>    state. Otherwise, [XA_RETRY] should be returned.
>
> However, since the pg_prepared_xacts table still contains a row for the transaction the XAER_RMERR is not accurate.
A"catastrophic" failure has not occurred.  It would be possible for the transaction manager to recover this transaction
oncethe database is available again if XA_RETRY was returned instead. 
>
> I think it would be better if commitPrepared could differentiate between errors and return either XAER_RMERR or
XA_RETRYas appropriate.  Otherwise just about any failure during commitPrepared will result in unrecoverable
transactionsand require manual intervention to clean up the pg_prepared_xacts table. 

Yes, good catch, we should be more careful to use the right error code.
For starters we could detect errors caused by failed connection, which
would include the case of server shutdown, and throw XA_RETRY. But I
guess we should be conservative here, and return XA_RETRY for anything
else than cases where we know for sure that the prepared transaction is
not there anymore. If COMMIT PREPARED is called for a non-existing
global transaction id, you get the error "prepared transaction with
identifier \"%s\" does not exist", with sqlstate 42704
(ERRCODE_UNDEFINED_OBJECT). We use ERRCODE_UNDEFINED_OBJECT for a lot of
things in the backend, but when you're issuing COMMIT PREPARED, I think
it's reasonable to assume that if you get that error code, the prepared
transaction is missing.

Attached patch implements that. I don't have a transaction manager
environment for this at hand right now, so this is completely untested.
Do you have a setup ready where you could test this?

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com