Re: Question regarding "Make archiver process an auxiliary process. commit"

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(dot)ntt(at)gmail(dot)com>
wrote:

> At Mon, 5 Dec 2022 12:06:11 +0530, Sravan Kumar <sravanvcybage(at)gmail(dot)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.