Re: Transactions involving multiple postgres foreign servers

On Sat, Aug 1, 2015 at 12:18 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Fri, Jul 31, 2015 at 6:33 AM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:>
> >> I'm not hung up on the table-level attribute, but I think having a
> >> server-level attribute rather than a global GUC is a good idea.
> >> However, I welcome other thoughts on that.
> >
> > The patch supports server level attribute. Let me repeat the relevant
> > description from my earlier mail
> > --
> > Every FDW needs to register the connection while starting new
> transaction on
> > a foreign connection (RegisterXactForeignServer()). A foreign server
> > connection is identified by foreign server oid and the local user oid
> > (similar to the entry cached by postgres_fdw). While registering, FDW
> also
> > tells whether the foreign server is capable of participating in two-phase
> > commit protocol. How to decide that is left entirely to the FDW. An FDW
> like
> > file_fdw may not have 2PC support at all, so all its foreign servers do
> not
> > comply with 2PC. An FDW might have all its servers 2PC compliant. An FDW
> > like postgres_fdw can have some of its servers compliant and some not,
> > depending upon server version, configuration (max_prepared_transactions
> = 0)
> > etc.
> > --
> >
> > Does that look good?
>
> OK, sure. But let's make sure postgres_fdw gets a server-level option
> to control this.
>
>
For postgres_fdw it's a boolean server-level option 'twophase_compliant'
(suggestions for name welcome).

> >> > Done, there are three hooks now
> >> > 1. For preparing a foreign transaction
> >> > 2. For resolving a prepared foreign transaction
> >> > 3. For committing/aborting a running foreign transaction (more
> >> > explanation
> >> > later)
> >>
> >> (2) and (3) seem like the same thing. I don't see any further
> >> explanation later in your email; what am I missing?
> >
> > In case of postgres_fdw, 2 always fires COMMIT/ROLLBACK PREPARED 'xyz'
> (fill
> > the prepared transaction id) and 3 always fires COMMIT/ABORT TRANSACTION
> > (notice absence of PREPARED and 'xyz').
>
> Oh, OK. But then isn't #3 something we already have? i.e.
> pgfdw_xact_callback?
>

While transactions are being prepared on the foreign connections, if any
prepare fails, we have to abort transactions on the rest of the connections
(and abort the prepared transactions). pgfdw_xact_callback wouldn't know,
which connections have prepared transactions and which do not have. So,
even in case of two-phase commit we need all the three hooks. Since we have
to define these three hooks, we might as well centralize all the
transaction processing and let the foreign transaction manager decide which
of the hooks to invoke. So, the patch moves most of the code in
pgfdw_xact_callback in the relevant hook and foreign transaction manager
invokes appropriate hook. Only thing that remains in pgfdw_xact_callback
now is end of transaction handling like resetting cursor numbering.

> >> That seems totally broken. Before PITR, the database might be
> >> inconsistent, in which case you can't call any functions at all.
> >> Also, you shouldn't be trying to resolve any transactions until the
> >> end of recovery, because you don't know when you see that the
> >> transaction was prepared whether, at some subsequent time, you will
> >> see it resolved. You need to finish recovery and, only after entering
> >> normal running, decide whether to resolve any transactions that are
> >> still sitting around.
> >
> > That's how it works in the patch for unresolved prepared foreign
> > transactions belonging to xids within the known range. For those
> belonging
> > to xids in future (beyond of known range of xids after PITR), we can not
> > determine the status of that local transaction (as those do not appear in
> > the xlog) and hence can not decide the fate of prepared foreign
> transaction.
> > You seem to be suggesting that we should let the recovery finish and mark
> > those prepared foreign transaction as "can not be resolved" or something
> > like that. A DBA can remove those entries once s/he has dealt with them
> on
> > the foreign server.
> >
> > There's little problem with that approach. Triplet (xid, serverid,
> userid)
> > is used to identify the a foreign prepared transaction entry in memory
> and
> > is used to create unique file name for storing it on the disk. If we
> allow a
> > future xid after PITR, it might conflict with an xid of a transaction
> that
> > might take place after PITR. It will cause problem if exactly same
> foreign
> > server and user participate in the transaction with conflicting xid (rare
> > but possible).
> >
> > Other problem is that the foreign server on which the transaction was
> > prepared (or the user whose mapping was used to prepare the transaction),
> > might have got added in a future time wrt PITR, in which case, we can not
> > even know which foreign server this transaction was prepared on.
> >
> >> There should be no situation (short of e.g. OS
> >> errors writing the state files) where this stuff makes recovery fail.
> >
> > During PITR, if we encounter a prepared (local) transaction with a future
> > xid, we just forget that prepared transaction (instead of failing
> recovery).
> > May be we should do the same for unresolved foreign prepared transaction
> as
> > well (at least for version 1); forget the unresolved prepared foreign
> > transactions which belong to a future xid. Anyway, as per the timeline
> after
> > PITR those never existed.
>
> This last sentence seems to me to be exactly on point. Note the
> comment in twophase.c:
>
> * We throw away any prepared xacts with main XID beyond nextXid --- if any
> * are present, it suggests that the DBA has done a PITR recovery to an
> * earlier point in time without cleaning out pg_twophase. We dare not
> * try to recover such prepared xacts since they likely depend on database
> * state that doesn't exist now.
>
> In other words, normally there should never be any XIDs "from the
> future" with prepared transactions; but in certain PITR scenarios it
> might be possible. We might as well be consistent with what the
> existing 2PC code does in this case - i.e. just warn and then remove
> the files.
>

Ok. Done.

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company