Re: I think we need PRE_COMMIT events for (Sub)XactCallbacks

Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> writes:
> I noticed you added a pre event for commit/prepare/subcommit. That
> looks good. Is there a case to add it for abort/subabort too ? I
> wonder if we would want to do some cleanup on the foreign servers
> before the transaction is abort-recorded on the main server.

I don't really think this is needed. The reason to have a pre-commit
step is to be able to throw an error and prevent commit from occurring.
In the abort case, there is no corresponding need to be able to change
the local transaction's result. So you can just do whatever you need
to do in the existing ABORT event. I'd just as soon not add overhead
to commit/abort without a demonstrated need.

> For
> example, if someone wants to implement a 2PC using transaction
> callbacks and need a mechanism to rollback prepared transactions
> because some foreign server refused to prepare, I'm not sure if she
> can use XACT_EVENT_ABORT because that callback is called while
> interrupts are disabled and so it may not be safe to communicate with
> the foreign servers.

[ shrug... ] An interrupt means "abort the current transaction and clean
up". If you're already trying to do that, it means nothing. It's not
like you could choose to not clean up. If we did have a pre-abort
event, throwing an error from it would just cause control to come right
back and do it again.

(Having said that, we will have to take a closer look at the
postgres_fdw code and make sure that it tries to avoid getting stuck at
ABORT time.)

> How about supporting all three modes such as 1. the current behaviour
> of immediately committing at the end of a statement, 2. a full 2PC and
> 3. what you are proposing.

Well, the current behavior is simply broken IMO, and no it's not any
faster --- you still need to send a commit/abort command, it's just
different timing. As for #2, fine, *you* implement that. I'm not
volunteering. First question is which XA manager you're going to assume
is controlling 2PC matters.

regards, tom lane