On Tue, May 2, 2023 at 9:46 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, May 2, 2023 at 9:06 AM Zhijie Hou (Fujitsu) > <houzj.f...@fujitsu.com> wrote: > > > > On Friday, April 28, 2023 2:18 PM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > > > > > > Alexander, does the proposed patch fix the problem you are facing? > > > > Sawada-San, and others, do you see any better way to fix it than what > > > > has been proposed? > > > > > > I'm concerned that the idea of relying on IsNormalProcessingMode() > > > might not be robust since if we change the meaning of > > > IsNormalProcessingMode() some day it would silently break again. So I > > > prefer using something like InitializingApplyWorker, or another idea > > > would be to do cleanup work (e.g., fileset deletion and lock release) > > > in a separate callback that is registered after connecting to the > > > database. > > > > Thanks for the review. I agree that it’s better to use a new variable here. > > Attach the patch for the same. > > > > + * > + * However, if the worker is being initialized, there is no need to release > + * locks. > */ > - LockReleaseAll(DEFAULT_LOCKMETHOD, true); > + if (!InitializingApplyWorker) > + LockReleaseAll(DEFAULT_LOCKMETHOD, true); > > Can we slightly reword this comment as: "The locks will be acquired > once the worker is initialized."? >
After making this modification, I pushed your patch. Thanks! -- With Regards, Amit Kapila.