MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Tue, 08 Sep 2020 20:00:44 +0300
From: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>,
 tsunakawa.takay@fujitsu.com, movead.li@highgo.ca, 'Amit Kapila'
 <amit.kapila16@gmail.com>, Masahiko Sawada
 <masahiko.sawada@2ndquadrant.com>, PostgreSQL-Dev
 <pgsql-hackers@postgresql.org>
Subject: Re: Global snapshots
In-Reply-To: <a86f7722-48b3-cd82-d491-86272a7bf8f0@oss.nttdata.com>
References: <07b2c899-4ed0-4c87-1327-23c750311248@postgrespro.ru>
 <f23083b9-38d0-6126-eb6e-091816a78585@postgrespro.ru>
 <CAA4eK1K8ibXN8vknTYvjz1sd+JaDDohvV9jBxSs+=1=KJz=u=w@mail.gmail.com>
 <CA+fd4k6oZtO-MFYmunHVecGaTWre8YKDNTSfX9hZhQh6Kui1kA@mail.gmail.com>
 <CAA4eK1JVsMWUD4q-b+vawehFzJb6Qg0AOGq7qOGL_gm6EEhdJg@mail.gmail.com>
 <CA+fd4k6HE8xLGEvqWzABEg8kkju5MxU+if7bf-md0_2pjzXp9Q@mail.gmail.com>
 <CAA4eK1+tMDXU9WtQ0kRgyk1jPr+K-7FowMdFB8=t-ptUEQa-mQ@mail.gmail.com>
 <CA+fd4k6YTa25VxpsqvpvqZmsCErT5T8=MYNNDXt7Wn0DsJvSrg@mail.gmail.com>
 <CAA4eK1KAB=YbvDd3a8g2qNj2QNrQANnrpNmoaNCtEMLfaiFjrg@mail.gmail.com>
 <TYAPR01MB2990E83F9E886A9C8AB8E269FE760@TYAPR01MB2990.jpnprd01.prod.outlook.com>
 <TYAPR01MB29902D8B671A5A6946F5AEA8FE720@TYAPR01MB2990.jpnprd01.prod.outlook.com>
 <806c3708-26ea-b94d-87b1-d48e41e93726@postgrespro.ru>
 <3ef7877bfed0582019eab3d462a43275@postgrespro.ru>
 <057004b6-68bf-98ee-7c57-91c418f6e221@oss.nttdata.com>
 <0e51d0298eed7588664e3c67a4fb15c9@postgrespro.ru>
 <a86f7722-48b3-cd82-d491-86272a7bf8f0@oss.nttdata.com>
User-Agent: Roundcube Webmail/1.4.0
Message-ID: <53b1586317ae98ecd8c3383f2c9e7c16@postgrespro.ru>
Precedence: bulk

On 2020-09-08 14:48, Fujii Masao wrote:
> On 2020/09/08 19:36, Alexey Kondratov wrote:
>> On 2020-09-08 05:49, Fujii Masao wrote:
>>> On 2020/09/05 3:31, Alexey Kondratov wrote:
>>>> 
>>>> Attached is a patch, which implements a plain 2PC in the 
>>>> postgres_fdw and adds a GUC 'postgres_fdw.use_twophase'. Also it 
>>>> solves these errors handling issues above and tries to add proper 
>>>> comments everywhere. I think, that 0003 should be rebased on the top 
>>>> of it, or it could be a first patch in the set, since it may be used 
>>>> independently. What do you think?
>>> 
>>> Thanks for the patch!
>>> 
>>> Sawada-san was proposing another 2PC patch at [1]. Do you have any 
>>> thoughts
>>> about pros and cons between your patch and Sawada-san's?
>>> 
>>> [1]
>>> https://www.postgresql.org/message-id/CA+fd4k4z6_B1ETEvQamwQhu4RX7XsrN5ORL7OhJ4B5B6sW-RgQ@mail.gmail.com
>> 
>> Thank you for the link!
>> 
>> After a quick look on the Sawada-san's patch set I think that there 
>> are two major differences:
> 
> Thanks for sharing your thought! As far as I read your patch quickly,
> I basically agree with your this view.
> 
> 
>> 
>> 1. There is a built-in foreign xacts resolver in the [1], which should 
>> be much more convenient from the end-user perspective. It involves 
>> huge in-core changes and additional complexity that is of course worth 
>> of.
>> 
>> However, it's still not clear for me that it is possible to resolve 
>> all foreign prepared xacts on the Postgres' own side with a 100% 
>> guarantee. Imagine a situation when the coordinator node is actually a 
>> HA cluster group (primary + sync + async replica) and it failed just 
>> after PREPARE stage of after local COMMIT. In that case all foreign 
>> xacts will be left in the prepared state. After failover process 
>> complete synchronous replica will become a new primary. Would it have 
>> all required info to properly resolve orphan prepared xacts?
> 
> IIUC, yes, the information required for automatic resolution is
> WAL-logged and the standby tries to resolve those orphan transactions
> from WAL after the failover. But Sawada-san's patch provides
> the special function for manual resolution, so there may be some cases
> where manual resolution is necessary.
> 

I've found a note about manual resolution in the v25 0002:

+After that we prepare all foreign transactions by calling
+PrepareForeignTransaction() API. If we failed on any of them we change 
to
+rollback, therefore at this time some participants might be prepared 
whereas
+some are not prepared. The former foreign transactions need to be 
resolved
+using pg_resolve_foreign_xact() manually and the latter ends 
transaction
+in one-phase by calling RollbackForeignTransaction() API.

but it's not yet clear for me.

> 
> Implementing 2PC feature only inside postgres_fdw seems to cause
> another issue; COMMIT PREPARED is issued to the remote servers
> after marking the local transaction as committed
> (i.e., ProcArrayEndTransaction()).
> 

According to the Sawada-san's v25 0002 the logic is pretty much the same 
there:

+2. Pre-Commit phase (1st phase of two-phase commit)

+3. Commit locally
+Once we've prepared all of them, commit the transaction locally.

+4. Post-Commit Phase (2nd phase of two-phase commit)

Brief look at the code confirms this scheme. IIUC, AtEOXact_FdwXact / 
FdwXactParticipantEndTransaction happens after ProcArrayEndTransaction() 
in the CommitTransaction(). Thus, I don't see many difference between 
these approach and CallXactCallbacks() usage regarding this point.

> Is this safe? This issue happens
> because COMMIT PREPARED is issued via
> CallXactCallbacks(XACT_EVENT_COMMIT) and that CallXactCallbacks()
> is called after ProcArrayEndTransaction().
> 

Once the transaction is committed locally any ERROR (or higher level 
message) will be escalated to PANIC. And I do see possible ERROR level 
messages in the postgresCommitForeignTransaction() for example:

+	if (PQresultStatus(res) != PGRES_COMMAND_OK)
+		ereport(ERROR, (errmsg("could not commit transaction on server %s",
+							   frstate->server->servername)));

I don't think that it's very convenient to get a PANIC every time we 
failed to commit one of the prepared foreign xacts, since it could be 
not so rare in the distributed system. That's why I tried to get rid of 
possible ERRORs as far as possible in my proposed patch.


Regards
-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company