Hi,
For v32-0004-Add-PrepareForeignTransaction-API.patch :
+ * Whenever a foreign transaction is processed, the corresponding FdwXact
+ * entry is update. To avoid holding the lock during transaction
processing
+ * which may take an unpredicatable time the in-memory data of foreign
entry is update -> entry is updated
unpredictable -> unpredictable
+ int nlefts = 0;
nlefts -> nremaining
+ elog(DEBUG1, "left %u foreign transactions", nlefts);
The message can be phrased as "%u foreign transactions remaining"
+FdwXactResolveFdwXacts(int *fdwxact_idxs, int nfdwxacts)
Fdw and Xact are repeated. Seems one should suffice. How about naming the
method FdwXactResolveTransactions() ?
Similar comment for FdwXactResolveOneFdwXact(FdwXact fdwxact)
For get_fdwxact():
+ /* This entry matches the condition */
+ found = true;
+ break;
Instead of breaking and returning, you can return within the loop directly.
Cheers
On Thu, Jan 14, 2021 at 9:17 PM Masahiko Sawada <sawada.m...@gmail.com>
wrote:
> On Fri, Jan 15, 2021 at 4:03 AM Zhihong Yu <z...@yugabyte.com> wrote:
> >
> > Hi,
> > For v32-0008-Prepare-foreign-transactions-at-commit-time.patch :
>
> Thank you for reviewing the patch!
>
> >
> > + bool have_notwophase = false;
> >
> > Maybe name the variable have_no_twophase so that it is easier to read.
>
> Fixed.
>
> >
> > + * Two-phase commit is not required if the number of servers
> performed
> >
> > performed -> performing
>
> Fixed.
>
> >
> > + errmsg("cannot process a distributed transaction that
> has operated on a foreign server that does not support two-phase commit
> protocol"),
> > + errdetail("foreign_twophase_commit is \'required\' but
> the transaction has some foreign servers which are not capable of two-phase
> commit")));
> >
> > The lines are really long. Please wrap into more lines.
>
> Hmm, we can do that but if we do that, it makes grepping by the error
> message hard. Please refer to the documentation about the formatting
> guideline[1]:
>
> Limit line lengths so that the code is readable in an 80-column
> window. (This doesn't mean that you must never go past 80 columns. For
> instance, breaking a long error message string in arbitrary places
> just to keep the code within 80 columns is probably not a net gain in
> readability.)
>
> These changes have been made in the local branch. I'll post the
> updated patch set after incorporating all the comments.
>
> Regards,
>
> [1] https://www.postgresql.org/docs/devel/source-format.html
>
> --
> Masahiko Sawada
> EnterpriseDB: https://www.enterprisedb.com/
>
