Re: Transactions involving multiple postgres foreign servers, take 2

On Fri, May 15, 2020 at 7:20 AM Masahiko Sawada <
masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:

> On Fri, 15 May 2020 at 03:08, Muhammad Usama <m(dot)usama(at)gmail(dot)com> wrote:
> >
> >
> > Hi Sawada,
> >
> > I have just done some review and testing of the patches and have
> > a couple of comments.
>
> Thank you for reviewing!
>
> >
> > 1- IMHO the PREPARE TRANSACTION should always use 2PC even
> > when the transaction has operated on a single foreign server regardless
> > of foreign_twophase_commit setting, and throw an error otherwise when
> > 2PC is not available on any of the data-modified servers.
> >
> > For example, consider the case
> >
> > BEGIN;
> > INSERT INTO ft_2pc_1 VALUES(1);
> > PREPARE TRANSACTION 'global_x1';
> >
> > Here since we are preparing the local transaction so we should also
> prepare
> > the transaction on the foreign server even if the transaction has
> modified only
> > one foreign table.
> >
> > What do you think?
>
> Good catch and I agree with you. The transaction should fail if it
> opened a transaction on a 2pc-no-support server regardless of
> foreign_twophase_commit. And I think we should prepare a transaction
> on a foreign server even if it didn't modify any data on that.
>
> >
> > Also without this change, the above test case produces an assertion
> failure
> > with your patches.
> >
> > 2- when deciding if the two-phase commit is required or not in
> > FOREIGN_TWOPHASE_COMMIT_PREFER mode we should use
> > 2PC when we have at least one server capable of doing that.
> >
> > i.e
> >
> > For FOREIGN_TWOPHASE_COMMIT_PREFER case in
> > checkForeignTwophaseCommitRequired() function I think
> > the condition should be
> >
> > need_twophase_commit = (nserverstwophase >= 1);
> > instead of
> > need_twophase_commit = (nserverstwophase >= 2);
> >
>
> Hmm I might be missing your point but it seems to me that you want to
> use two-phase commit even in the case where a transaction modified
> data on only one server. Can't we commit distributed transaction
> atomically even using one-phase commit in that case?
>
>
I think you are confusing between nserverstwophase and nserverswritten.

need_twophase_commit = (nserverstwophase >= 1) would mean
use two-phase commit if at least one server exists in the list that is
capable of doing 2PC

For the case when the transaction modified data on only one server we
already exits the function indicating no two-phase required

if (nserverswritten <= 1)
return false;

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

Regards,
...
Muhammad Usama
Highgo Software (Canada/China/Pakistan)
URL : http://www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC