I think we need PRE_COMMIT events for (Sub)XactCallbacks

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: I think we need PRE_COMMIT events for (Sub)XactCallbacks
Date: 2013-02-14 23:49:48
Message-ID: 1142.1360885788@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 are done ... 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-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.

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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2013-02-15 00:02:28 Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system
Previous Message Tomas Vondra 2013-02-14 23:48:02 Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system