Thread: I think we need PRE_COMMIT events for (Sub)XactCallbacks
I'm trying to whack the postgres_fdw patch into committable shape, with
one eye on the writable-foreign-tables patch that's right behind it in
the queue. One thing I've come across is that the timing of remote
commits is a mess. As-is in the submitted patches, we'll issue a commit
to the remote server as soon as the executor shuts down a query. I'm
not too thrilled with this, since given
begin;update my_foreign_table set ... ;update my_foreign_table set ... ;update my_foreign_table set ... ;rollback;
a reasonable person would expect that the remote updates are rolled
back. But as it stands each UPDATE is committed instantly on
completion.
The only facility that postgres_fdw would have for doing it differently
is to plug into the XactCallback or ResourceReleaseCallback hooks, and
the problem with both of those is they are post-commit hooks, which
means it's too late to throw an error. Now foreign-side errors at
commit are not exactly difficult to foresee --- for instance, one of
our updates might have violated a deferred foreign-key constraint on the
remote side, and we won't hear about that till we try to commit. If we
then report that error in one of the existing hooks, it'll become a
PANIC. No good.
So I think we need to add a pre-commit event to the set of events that
XactCallbacks are called for, probably at this spot in CommitTransaction:
/* * Close open portals (converting holdable ones into static portals). * If there weren't any, we
aredone ... otherwise loop back to check * if they queued deferred triggers. Lather, rinse, repeat. */
if (!PreCommit_Portals(false)) break; }
+ CallXactCallbacks(XACT_EVENT_PRE_COMMIT);
+ /* * The remaining actions cannot call any user-defined code, so it's safe * to start shutting down
within-transactionservices. But note that most * of this stuff could still throw an error, which would switch us
into * the transaction-abort path. */
and similarly in PrepareTransaction. We're probably also going to need
a pre-subcommit event in CommitSubTransaction --- maybe we could get
away without that, but I'm not convinced, and we might as well change
both the XactCallback and SubXactCallback APIs at the same time.
Note that this doesn't come near a true two-phase-commit protocol;
there will still be a window wherein we've done COMMIT on the remote
side but the local side could fail and decide to roll back. However,
the possible errors in that window are limited and unusual, so it's
not clear to me that it's worth working harder than this.
Any objections?
regards, tom lane
On 14 February 2013 23:49, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Any objections? Makes sense to me. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Feb 15, 2013 at 5:19 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > + CallXactCallbacks(XACT_EVENT_PRE_COMMIT); > + > /* > * The remaining actions cannot call any user-defined code, so it's safe > * to start shutting down within-transaction services. But note that most > * of this stuff could still throw an error, which would switch us into > * the transaction-abort path. > */ > > and similarly in PrepareTransaction. We're probably also going to need > a pre-subcommit event in CommitSubTransaction --- maybe we could get > away without that, but I'm not convinced, and we might as well change > both the XactCallback and SubXactCallback APIs at the same time. > 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. 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. > Note that this doesn't come near a true two-phase-commit protocol; > there will still be a window wherein we've done COMMIT on the remote > side but the local side could fail and decide to roll back. However, > the possible errors in that window are limited and unusual, so it's > not clear to me that it's worth working harder than this. > 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. The first will be fast because you don't need additional round-trip at commit/abort but is terrible WRT data consistency. The second is most reliable, but will have increased transaction commit time. The third is a nice balance and can be the default. I understand this might be too much work for now. But it will awesome if we can have all machinery in place to support these configurable modes in future. Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee
Pavan Deolasee <pavan.deolasee@gmail.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