At Mon, 5 Dec 2022 12:06:11 +0530, Sravan Kumar <sravanvcybage@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
