Thank you for the feedback. I'll be glad to help with the fix. Here's the patch for review.
On Tue, Dec 6, 2022 at 1:54 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > At Mon, 5 Dec 2022 12:06:11 +0530, Sravan Kumar <sravanvcyb...@gmail.com> > wrote in > > timeout = PGARCH_AUTOWAKE_INTERVAL - (curtime - last_copy_time); > > It so happens that last_copy_time and curtime are always set at the same > > time which always makes timeout equal (actually roughly equal) to > > PGARCH_AUTOWAKE_INTERVAL. > > Oooo *^^*. > > > This behaviour was different before the commit: > > d75288fb27b8fe0a926aaab7d75816f091ecdc27, > > in which the archiver keeps track of how much time has elapsed since > > last_copy_time > > in case there was a signal, and it results in a smaller subsequent value > of > > timeout, until timeout is zero. This also avoids calling > > pgarch_ArchiverCopyLoop > > before PGARCH_AUTOWAKE_INTERVAL in case there's an intermittent signal. > > Yes, WaitLatch() (I believe) no longer makes a spurious wakeup. > > > With the current changes it may be okay to always sleep for > > PGARCH_AUTOWAKE_INTERVAL, > > but that means curtime and last_copy_time are no more needed. > > I think you're right. > > > I would like to validate if my understanding is correct, and which of the > > behaviours we would like to retain. > > As my understanding the patch didn't change the copying behavior of > the function. I think we should simplify the loop by removing > last_copy_time and curtime in the "if (!time_to_stop)" block. Then we > can remove the variable "timeout" and the "if (timeout > 0)" > branch. Are you willing to work on this? > > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center > -- Thanks And Regards, Sravan Take life one day at a time.
v1-0001-simplify-wait-loop-in-the-archiver.patch
Description: Binary data
