On Fri, 15 May 2020 at 03:08, Muhammad Usama <m.us...@gmail.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? Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
