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
