(I finally get to catch up here..) At Mon, 22 Mar 2021 13:59:47 +0900, Fujii Masao <masao.fu...@oss.nttdata.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
