From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
Cc: tsunakawa.takay@fujitsu.com, movead.li@highgo.ca, 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: PostgreSQL-Dev <pgsql-hackers@postgresql.org>
Subject: Re: Global snapshots
Date: Tue, 8 Sep 2020 11:49:36 +0900
Message-ID: <057004b6-68bf-98ee-7c57-91c418f6e221@oss.nttdata.com> (raw)
In-Reply-To: <3ef7877bfed0582019eab3d462a43275@postgrespro.ru>
References: <07b2c899-4ed0-4c87-1327-23c750311248@postgrespro.ru>
<9964cf46-9294-34b9-4858-971e9029f5c7@postgrespro.ru>
<CAA4eK1Jo=1261+qEKmm69Bm9wv6fvR7kVbqDo7EFUw3vkQtC7w@mail.gmail.com>
<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>
On 2020/09/05 3:31, Alexey Kondratov wrote:
> Hi,
>
> On 2020-07-27 09:44, Andrey V. Lepikhov wrote:
>> On 7/27/20 11:22 AM, tsunakawa.takay@fujitsu.com wrote:
>>>
>>> US8356007B2 - Distributed transaction management for database systems with multiversioning - Google Patents
>>> https://patents.google.com/patent/US8356007
>>>
>>>
>>> If it is, can we circumvent this patent?
>>>
>>
>> Thank you for the research (and previous links too).
>> I haven't seen this patent before. This should be carefully studied.
>
> I had a look on the patch set, although it is quite outdated, especially on 0003.
>
> Two thoughts about 0003:
>
> First, IIUC atomicity of the distributed transaction in the postgres_fdw is achieved by the usage of 2PC. I think that this postgres_fdw 2PC support should be separated from global snapshots.
Agreed.
> It could be useful to have such atomic distributed transactions even without a proper visibility, which is guaranteed by the global snapshot. Especially taking into account the doubts about Clock-SI and general questions about algorithm choosing criteria above in the thread.
>
> Thus, I propose to split 0003 into two parts and add a separate GUC 'postgres_fdw.use_twophase', which could be turned on independently from 'postgres_fdw.use_global_snapshots'. Of course if the latter is enabled, then 2PC should be forcedly turned on as well.
>
> Second, there are some problems with errors handling in the 0003 (thanks to Arseny Sher for review).
>
> +error:
> + if (!res)
> + {
> + sql = psprintf("ABORT PREPARED '%s'", fdwTransState->gid);
> + BroadcastCmd(sql);
> + elog(ERROR, "Failed to PREPARE transaction on remote node");
> + }
>
> It seems that we should never reach this point, just because BroadcastStmt will throw an ERROR if it fails to prepare transaction on the foreign server:
>
> + if (PQresultStatus(result) != expectedStatus ||
> + (handler && !handler(result, arg)))
> + {
> + elog(WARNING, "Failed command %s: status=%d, expected status=%d", sql, PQresultStatus(result), expectedStatus);
> + pgfdw_report_error(ERROR, result, entry->conn, true, sql);
> + allOk = false;
> + }
>
> Moreover, It doesn't make much sense to try to abort prepared xacts, since if we failed to prepare it somewhere, then some foreign servers may become unavailable already and this doesn't provide us a 100% guarantee of clean up.
>
> + /* COMMIT open transaction of we were doing 2PC */
> + if (fdwTransState->two_phase_commit &&
> + (event == XACT_EVENT_PARALLEL_COMMIT || event == XACT_EVENT_COMMIT))
> + {
> + BroadcastCmd(psprintf("COMMIT PREPARED '%s'", fdwTransState->gid));
> + }
>
> At this point, the host (local) transaction is already committed and there is no way to abort it gracefully. However, BroadcastCmd may rise an ERROR that will cause a PANIC, since it is non-recoverable state:
>
> PANIC: cannot abort transaction 487, it was already committed
>
> 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?
Regards,
[1]
https://www.postgresql.org/message-id/CA+fd4k4z6_B1ETEvQamwQhu4RX7XsrN5ORL7OhJ4B5B6sW-RgQ@mail.gmail...
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, a.kondratov@postgrespro.ru, a.lepikhov@postgrespro.ru, amit.kapila16@gmail.com, masahiko.sawada@2ndquadrant.com
Subject: Re: Global snapshots
In-Reply-To: <057004b6-68bf-98ee-7c57-91c418f6e221@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox