On Tue, Dec 19, 2023 at 6:35 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> ====
> walsender.c
>
> 01. WalSndWaitForStandbyConfirmation
>
> ```
> + sleeptime = WalSndComputeSleeptime(GetCurrentTimestamp());
> ```
>
> It works well, but I'm not sure whether we should use WalSndComputeSleeptime()
> because the function won't be called by walsender.
>
I don't think it is correct to use this function because it is
walsender specific, for example, it uses 'last_reply_timestamp' which
won't be even initialized in the backend environment. We need to
probably use a different logic for sleep here or need to use a
hard-coded value. I think we should change the name of functions like
WalSndWaitForStandbyConfirmation() as they are no longer used by
walsender. IIRC, earlier, we had a common logic to wait from both
walsender and SQL APIs which led to this naming but that is no longer
true with the latest patch.
> 02.WalSndWaitForStandbyConfirmation
>
> ```
> + ConditionVariableTimedSleep(&WalSndCtl->wal_confirm_rcv_cv, sleeptime,
> + WAIT_EVENT_WAL_SENDER_WAIT_FOR_STANDBY_CONFIRMATION)
> ```
>
> Hmm, is it OK to use the same event as WalSndWaitForWal()? IIUC it should be avoided.
>
Agreed. So, how about using WAIT_EVENT_WAIT_FOR_STANDBY_CONFIRMATION
so that we can use it both from the backend and walsender?
--
With Regards,
Amit Kapila.