Re: Transactions involving multiple postgres foreign servers, take 2

On Wed, 17 Jun 2020 at 09:01, Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> wrote:
>
> > I've attached the new version patch set. 0006 is a separate patch
> > which introduces 'prefer' mode to foreign_twophase_commit.
>
> I hope we can use this feature. Thank you for making patches and
> discussions.
> I'm currently understanding the logic and found some minor points to be
> fixed.
>
> I'm sorry if my understanding is wrong.
>
> * The v22 patches need rebase as they can't apply to the current master.
>
> * FdwXactAtomicCommitParticipants said in
> src/backend/access/fdwxact/README
> is not implemented. Is FdwXactParticipants right?

Right.

>
> * A following comment says that this code is for "One-phase",
> but second argument of FdwXactParticipantEndTransaction() describes
> this code is not "onephase".
>
> AtEOXact_FdwXact() in fdwxact.c
> /* One-phase rollback foreign transaction */
> FdwXactParticipantEndTransaction(fdw_part, false, false);
>
> static void
> FdwXactParticipantEndTransaction(FdwXactParticipant *fdw_part, bool
> onephase,
> bool for_commit)
>
> * "two_phase_commit" option is mentioned in postgres-fdw.sgml,
> but I can't find related code.
>
> * resolver.c comments have the sentence
> containing two blanks.(Emergency Termination)
>
> * There are some inconsistency with PostgreSQL wiki.
> https://wiki.postgresql.org/wiki/Atomic_Commit_of_Distributed_Transactions
>
> I understand it's difficult to keep consistency, I think it's ok to
> fix later
> when these patches almost be able to be committed.
>
> - I can't find "two_phase_commit" option in the source code.
> But 2PC is work if the remote server's "max_prepared_transactions"
> is set
> to non zero value. It is correct work, isn't it?

Yes. I had removed two_phase_commit option from postgres_fdw.
Currently, postgres_fdw uses 2pc when 2pc is required. Therefore,
max_prepared_transactions needs to be set to more than one, as you
mentioned.

>
> - some parameters are renamed or added in latest patches.
> max_prepared_foreign_transaction, max_prepared_transactions and so
> on.
>
> - typo: froeign_transaction_resolver_timeout
>

Thank you for your review! I've incorporated your comments on the
local branch. I'll share the latest version patch.

Also, I've updated the wiki page. I'll try to keep the wiki page up-to-date.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services