Re: Type of wait events WalReceiverWaitStart and WalSenderWaitForWAL

(I finally get to catch up here..)

At Mon, 22 Mar 2021 13:59:47 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>
>
> On 2021/03/22 12:01, Kyotaro Horiguchi wrote:
> > Mmm. I agree that it waits for WAL in most cases, but still WAL-wait
> > is activity for me because it is not waiting for being cued by
> > someone, but waiting for new WAL to come to perform its main purpose.
> > If it's an IPC, all waits on other than pure sleep should fall into
> > IPC? (I was confused by the comment of WalSndWait, which doesn't
> > state that it is waiting for latch..)
> > Other point I'd like to raise is that the client_wait case should be
> > distinctive from the WAL-wait since it is significant sign of what is
> > happening.
> > So I propose two chagnes here.
> > a. Rewrite the comment of WalSndWait so that it states that "also
> > waiting for latch-set".
>
> +1

Cool.

> > b. Split the event to two different events.
> > - WalSndWait(wakeEvents, sleeptime, WAIT_EVENT_WAL_SENDER_MAIN);
> > + WalSndWait(wakeEvents, sleeptime,
> > + pq_is_send_pending() ? WAIT_EVENT_WAL_SENDER_WRITE_DATA:
> > + WAIT_EVENT_WAL_SENDER_MAIN);
> > And _WRITE_DATA as client_wait and _SENDER_MAIN as activity.
> > What do you think about this?
>
> I'm ok with this. What about the attached patch
> (WalSenderWriteData.patch)?

Yeah, that is better. I'm fine with it as a whole.

+ * Overwrite wait_event with WAIT_EVENT_WAL_SENDER_WRITE_DATA
+ * if we have pending data in the output buffer and are waiting to write
+ * data to a client.

Since the function doesn't check for that directly, I'd like to write
as the following.

Overwrite wait_event with WAIT_EVENT_WAL_SENDER_WRITE_DATA if the
caller told to wait for WL_SOCKET_WRITEABLE, which means that we have
pending data in the output buffer and are waiting to write data to a
client.

> > Yes. The WAIT_EVENT_WAL_SENDER_WAIT_WAL is equivalent to
> > WAIT_EVENT_WAL_SENDER_MAIN as function. So I think it should be in
> > the same category as WAIT_EVENT_WAL_SENDER_MAIN. And like the 1 above,
> > wait_client case should be distinctive from the _MAIN event.
>
> +1. What about the attached patch (WalSenderWaitForWAL.patch)?

Looks good to me. Thanks.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center