Re: Transactions involving multiple postgres foreign servers, take 2

On Wed, Jun 3, 2020 at 12:02 PM Masahiko Sawada
<masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
>
> On Wed, 3 Jun 2020 at 14:50, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> >
> > If the intention is to keep the first version simple, then why do we
> > want to support any mode other than 'required'? I think it will limit
> > its usage for the cases where 2PC can be used only when all FDWs
> > involved support Prepare API but if that helps to keep the design and
> > patch simpler then why not just do that for the first version and then
> > extend it later. OTOH, if you think it will be really useful to keep
> > other modes, then also we could try to keep those in separate patches
> > to facilitate the review and discussion of the core feature.
>
> ‘disabled’ is the fundamental mode. We also need 'disabled' mode,
> otherwise existing FDW won't work.
>

IIUC, if foreign_twophase_commit is 'disabled', we don't use a
two-phase protocol to commit distributed transactions, right? So, do
we check this at the time of Prepare or Commit whether we need to use
a two-phase protocol? I think this should be checked at prepare time.

+ <para>
+ This parameter can be changed at any time; the behavior for any one
+ transaction is determined by the setting in effect when it commits.
+ </para>

This is written w.r.t foreign_twophase_commit. If one changes this
between prepare and commit, will it have any impact?

> I was concerned that many FDW
> plugins don't implement FDW transaction APIs yet when users start
> using this feature. But it seems to be a good idea to move 'prefer'
> mode to a separate patch while leaving 'required'. I'll do that in the
> next version patch.
>

Okay, thanks. Please, see if you can separate out the documentation
for that as well.

Few other comments on v21-0003-Documentation-update:
----------------------------------------------------
1.
+ <entry></entry>
+ <entry>
+ Numeric transaction identifier with that this foreign transaction
+ associates
+ </entry>

/with that this/with which this

2.
+ <entry>
+ The OID of the foreign server on that the foreign transaction
is prepared
+ </entry>

/on that the/on which the

3.
+ <entry><structfield>status</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry></entry>
+ <entry>
+ Status of foreign transaction. Possible values are:
+ <itemizedlist>
+ <listitem>
+ <para>
+ <literal>initial</literal> : Initial status.
+ </para>

What exactly "Initial status" means?

4.
+ <entry><structfield>in_doubt</structfield></entry>
+ <entry><type>boolean</type></entry>
+ <entry></entry>
+ <entry>
+ If <literal>true</literal> this foreign transaction is
in-doubt status and
+ needs to be resolved by calling <function>pg_resolve_fdwxact</function>
+ function.
+ </entry>

It would be better if you can add an additional sentence to say when
and or how can foreign transactions reach in-doubt state.

5.
If <literal>N</literal> local transactions each
+ across <literal>K</literal> foreign server this value need to be set

This part of the sentence can be improved by saying something like:
"If a user expects N local transactions and each of those involves K
foreign servers, this value..".

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com