On Mon, Sep 12, 2022 at 4:27 PM kuroda.hay...@fujitsu.com <kuroda.hay...@fujitsu.com> wrote: > > Dear Hou-san, > > Thank you for updating the patch! Followings are comments for v28-0001. > I will dig your patch more, but I send partially to keep the activity of the > thread. > > === > For applyparallelworker.c > > 01. filename > The word-ordering of filename seems not good > because you defined the new worker as "parallel apply worker". >
I think in the future we may have more files for apply work (like applyddl.c for DDL apply work), so it seems okay to name all apply related files in a similar way. > > === > For worker.c > > 07. general > > In many lines if-else statement is used for apply_action, but I think they > should rewrite as switch-case statement. > Sounds reasonable to me. > 08. global variable > > ``` > -static bool in_streamed_transaction = false; > +bool in_streamed_transaction = false; > ``` > > a. > > It seems that in_streamed_transaction is used only in the worker.c, so we can > change to stati variable. > Yeah, I don't know why it has been changed in the first place. > b. > > That flag is set only when an apply worker spill the transaction to the disk. > How about "in_streamed_transaction" -> "in_spilled_transaction"? > Isn't this an existing variable? If so, it doesn't seem like a good idea to change the name unless we are changing its meaning. -- With Regards, Amit Kapila.
