On Tue, Dec 19, 2023 at 6:35 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@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.
