On Sat, Jan 16, 2021 at 1:39 AM Zhihong Yu <z...@yugabyte.com> wrote: > > Hi,
Thank you for reviewing the patch! > 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 Fixed. ยจ > > + int nlefts = 0; > > nlefts -> nremaining > > + elog(DEBUG1, "left %u foreign transactions", nlefts); > > The message can be phrased as "%u foreign transactions remaining" Fixed. > > +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) Agreed. I changed to ResolveFdwXacts() and ResolveOneFdwXact() respectively to avoid a long function name. > > For get_fdwxact(): > > + /* This entry matches the condition */ > + found = true; > + break; > > Instead of breaking and returning, you can return within the loop directly. Fixed. Those changes are incorporated into the latest version patches[1] I submitted today. Regards, [1] https://www.postgresql.org/message-id/CAD21AoBYyA5O%2BFPN4Cs9YWiKjq319BvF5fYmKNsFTZfwTcWjQw%40mail.gmail.com -- Masahiko Sawada EDB: https://www.enterprisedb.com/
