Hello.
# It took a long time to come here..
At Fri, 19 Oct 2018 21:38:35 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in
<CAD21AoCBf-AJup-_ARfpqR42gJQ_XjNsvv-XE0rCOCLEkT=HCg@mail.gmail.com>
> On Wed, Oct 10, 2018 at 1:34 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
...
> * Updated docs, added the new section "Distributed Transaction" at
> Chapter 33 to explain the concept to users
>
> * Moved atomic commit codes into src/backend/access/fdwxact directory.
>
> * Some bug fixes.
>
> Please reivew them.
I have some comments, with apologize in advance for possible
duplicate or conflict with others' comments so far.
0001:
This sets XACT_FLAG_WROTENONTEMPREL when RELPERSISTENT_PERMANENT
relation is modified. Isn't it needed when UNLOGGED tables are
modified? It may be better that we have dedicated classification
macro or function.
The flag is handled in heapam.c. I suppose that it should be done
in the upper layer considering coming pluggable storage.
(X_F_ACCESSEDTEMPREL is set in heapam, but..)
0002:
The name FdwXactParticipantsForAC doesn't sound good for me. How
about FdwXactAtomicCommitPartitcipants?
Well, as the file comment of fdwxact.c,
FdwXactRegisterTransaction is called from FDW driver and
F_X_MarkForeignTransactionModified is called from executor. I
think that we should clarify who is responsible to the whole
sequence. Since the state of local tables affects, I suppose
executor is that. Couldn't we do the whole thing within executor
side? I'm not sure but I feel that
F_X_RegisterForeignTransaction can be a part of
F_X_MarkForeignTransactionModified. The callers of
MarkForeignTransactionModified can find whether the table is
involved in 2pc by IsTwoPhaseCommitEnabled interface.
> if (foreign_twophase_commit == true &&
> ((MyXactFlags & XACT_FLAGS_FDWNOPREPARE) != 0) )
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot COMMIT a distributed transaction that has operated on foreign server that doesn't
supportatomic commit")));
The error is emitted when a the GUC is turned off in the
trasaction where MarkTransactionModify'ed. I think that the
number of the variables' possible states should be reduced for
simplicity. For example in the case, once foreign_twopase_commit
is checked in a transaction, subsequent changes in the
transaction should be ignored during the transaction.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
