Re: Possible oversight in org.postgresql.xa.PGXAConnection.commitPrepared(Xid xid)

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

Justin

----- Original Message -----
From: "Heikki Linnakangas" <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: "Justin Bertram" <jbertram(at)redhat(dot)com>
Cc: pgsql-jdbc(at)postgresql(dot)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(Xid xid). The driver will throw an XAException with an errorCode of XAER_RMERR back 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 its call to javax.transaction.xa.XAResource.commit(..) [1]. This is the JTA mapping of the xa_commit() function from the XA 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 once the 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_RETRY as appropriate. Otherwise just about any failure during commitPrepared will result in unrecoverable transactions and 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