Thread: Re: Type of wait events WalReceiverWaitStart and WalSenderWaitForWAL
On 2021/03/17 15:31, Kyotaro Horiguchi wrote: > I think it'd be better that they are categorized by what it is waiting > for. Yes. And some processes can be waiting for several events at the same moment. In this case we should pick the event that those proceses *mainly* are waiing for, as a wait event, I think. > Activity is waiting for something gating me to be released. > > IPC is waiting for the response for a request previously sent to > another process. > > Wait-client is waiting for the peer over a network connection to allow > me to proceed activity. I'm not sure if these definitions are really right or not because they seem to be slightly different from those in the document. > So whether the three fall into the same category or not doesn't matter > to me. Understood. > WAIT_EVENT_WAL_RECEIVER_MAIN(WalReceiverMain) is waiting for new data > to arrive. This looks like an activity to me. +1. So our consensus is not to change the category of this event. > WAIT_EVENT_WAL_RECEIVER_WAIT_START is waiting for waiting for starup > process to kick me. So it may be either IPC or Activity. Since > walreceiver hasn't sent anything to startup, so it's activity, rather > than IPC. However, the behavior can be said that it convey a piece of > information from startup to wal receiver so it also can be said to be > an IPC. (That is the reason why I don't object for IPC.) IMO this should be IPC because walreceiver is mainly waiting for the interaction with the startup process, during this wait event. Since you can live with IPC, probably our consensus is to use IPC? > 1(WAIT_EVENT_WAL_SENDER_MAIN, currently an activity) is waiting for > something to happen on the connection to the peer > receiver/worker. This might either be an activity or an wait_client, > but I prefer it to be wait_client, as the same behavior of a client > backend is categorizes as wait_client. Yes, walsender is waiting for replies from the standby to arrive during this event. But I think that it's *mainly* waiting for WAL to be flushed in order to send it. So IPC is better for this event rather than Client. On the other hand, wait events reported in main loop are basically categorized in Activity, in other processes. So in the sake of consistency, I like Activity rather than IPC, for this event. > 2 (WAIT_EVENT_WAL_SENDER_WRITE_DATA, currently a wait_client) is the > same to 1. IIUC walsender is mainly waiting for the socket to be writeable, to send any pending data. So I agree to use Client for this event. Our consensus seems not to change the category of this event. > 3 (WAIT_EVENT_WAL_SENDER_WAIT_WAL, currently a wait_client) is the > same to 1. Yes, walsender is waiting for replies from the standby to arrive during this event. But I think that it's *mainly* waiting for WAL to be flushed in order to send it. So IPC is better for this event rather than Client. On the other hand, while the server is in idle, this event is reported for logical walsender. This makes me think that it might be Activity, i.e., we should treat this as the wait event in logical walsender's main loop. So I like Activity rather than IPC, for this event. If we do this, it might be better to rename the event to WAIT_EVENT_LOGICAL_SENDER_MAIN. Therefore, my current idea is WAIT_EVENT_WAL_RECEIVER_MAIN should be in Activity (as currently it is) WAIT_EVENT_WAL_RECEIVER_WAIT_START should be moved to in IPC WAIT_EVENT_WAL_SENDER_MAIN should be in Activity (as currently it is) WAIT_EVENT_WAL_SENDER_WRITE_DATA should be in Client (as currently it is) WAIT_EVENT_WAL_SENDER_WAIT_WAL should be moved to in Activity. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On 2021/03/18 18:48, Fujii Masao wrote: >> WAIT_EVENT_WAL_RECEIVER_WAIT_START is waiting for waiting for starup >> process to kick me. So it may be either IPC or Activity. Since >> walreceiver hasn't sent anything to startup, so it's activity, rather >> than IPC. However, the behavior can be said that it convey a piece of >> information from startup to wal receiver so it also can be said to be >> an IPC. (That is the reason why I don't object for IPC.) > > IMO this should be IPC because walreceiver is mainly waiting for the > interaction with the startup process, during this wait event. Since you can > live with IPC, probably our consensus is to use IPC? If this is ok, I'd like to apply the attached patch at first. This patch changes the type of WAIT_EVENT_WAL_RECEIVER_WAIT_START from Client to IPC. BTW, I found that recently WalrcvExit wait event was introduced. But this name is not consistent with other events. I'm thinking that it's better to rename it to WalReceiverExit. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
At Thu, 18 Mar 2021 18:48:50 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in > On 2021/03/17 15:31, Kyotaro Horiguchi wrote: > > I think it'd be better that they are categorized by what it is waiting > > for. > > Yes. And some processes can be waiting for several events at the same > moment. In this case we should pick the event that those proceses > *mainly* are waiing for, as a wait event, I think. Right. > > Activity is waiting for something gating me to be released. > > IPC is waiting for the response for a request previously sent to > > another process. > > Wait-client is waiting for the peer over a network connection to allow > > me to proceed activity. > > I'm not sure if these definitions are really right or not because they > seem to be slightly different from those in the document. Maybe it depends on what "main processing loop" means. I found my words are inaccurate. "something gating me" meant that the main work. In the case of walsender main loop, it's advance of WAL flush location. In a broader idea it is a kind of IPC in most cases but the difference is, as you daid, in what the wait is waiting in those cases. > > So whether the three fall into the same category or not doesn't matter > > to me. > > Understood. > > > > WAIT_EVENT_WAL_RECEIVER_MAIN(WalReceiverMain) is waiting for new data > > to arrive. This looks like an activity to me. > > +1. So our consensus is not to change the category of this event. Agreed. > > WAIT_EVENT_WAL_RECEIVER_WAIT_START is waiting for waiting for starup > > process to kick me. So it may be either IPC or Activity. Since > > walreceiver hasn't sent anything to startup, so it's activity, rather > > than IPC. However, the behavior can be said that it convey a piece of > > information from startup to wal receiver so it also can be said to be > > an IPC. (That is the reason why I don't object for IPC.) > > IMO this should be IPC because walreceiver is mainly waiting for the > interaction with the startup process, during this wait event. Since > you can > live with IPC, probably our consensus is to use IPC? Exactly. > > 1(WAIT_EVENT_WAL_SENDER_MAIN, currently an activity) is waiting for > > something to happen on the connection to the peer > > receiver/worker. This might either be an activity or an wait_client, > > but I prefer it to be wait_client, as the same behavior of a client > > backend is categorizes as wait_client. > > Yes, walsender is waiting for replies from the standby to arrive > during > this event. But I think that it's *mainly* waiting for WAL to be > flushed > in order to send it. So IPC is better for this event rather than > Client. > On the other hand, wait events reported in main loop are basically > categorized in Activity, in other processes. So in the sake of > consistency, > I like Activity rather than IPC, for this event. 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". 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? > > 2 (WAIT_EVENT_WAL_SENDER_WRITE_DATA, currently a wait_client) is the > > same to 1. > > IIUC walsender is mainly waiting for the socket to be writeable, to > send > any pending data. So I agree to use Client for this event. Our > consensus > seems not to change the category of this event. Right. > > 3 (WAIT_EVENT_WAL_SENDER_WAIT_WAL, currently a wait_client) is the > > same to 1. > > Yes, walsender is waiting for replies from the standby to arrive > during > this event. But I think that it's *mainly* waiting for WAL to be > flushed > in order to send it. So IPC is better for this event rather than > Client. > > On the other hand, while the server is in idle, this event is reported > for > logical walsender. This makes me think that it might be Activity, > i.e., > we should treat this as the wait event in logical walsender's main > loop. > So I like Activity rather than IPC, for this event. > If we do this, it might be better to rename the event to > WAIT_EVENT_LOGICAL_SENDER_MAIN. 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. > Therefore, my current idea is > > WAIT_EVENT_WAL_RECEIVER_MAIN should be in Activity (as currently it > is) > WAIT_EVENT_WAL_RECEIVER_WAIT_START should be moved to in IPC > WAIT_EVENT_WAL_SENDER_MAIN should be in Activity (as currently it is) > WAIT_EVENT_WAL_SENDER_WRITE_DATA should be in Client (as currently it > is) > WAIT_EVENT_WAL_SENDER_WAIT_WAL should be moved to in Activity. Mine is. > WAIT_EVENT_WAL_RECEIVER_MAIN should be in Activity (as currently it > is) > WAIT_EVENT_WAL_RECEIVER_WAIT_START should be moved to in IPC Agreed. > WAIT_EVENT_WAL_SENDER_MAIN should be in Activity (as currently it is) > WAIT_EVENT_WAL_SENDER_WRITE_DATA should be in Client (as currently it > is) > WAIT_EVENT_WAL_SENDER_WAIT_WAL should be moved to in Activity. Agreed. And I'd like to add _SENDER_WRITE_DATA as the alternative event for _SENDER_MAIN in the case pq_is_send_pending() == true. And also I'd like to propose edit the comment of WalSndWait(). regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On 2021/03/22 12:01, Kyotaro Horiguchi wrote: >>> WAIT_EVENT_WAL_RECEIVER_WAIT_START is waiting for waiting for starup >>> process to kick me. So it may be either IPC or Activity. Since >>> walreceiver hasn't sent anything to startup, so it's activity, rather >>> than IPC. However, the behavior can be said that it convey a piece of >>> information from startup to wal receiver so it also can be said to be >>> an IPC. (That is the reason why I don't object for IPC.) >> >> IMO this should be IPC because walreceiver is mainly waiting for the >> interaction with the startup process, during this wait event. Since >> you can >> live with IPC, probably our consensus is to use IPC? > > Exactly. Ok, so barring any objection, I will commit the patch that I posted upthread. > 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 > 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)? > 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)? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
On 2021/03/22 13:59, Fujii Masao wrote: > > Ok, so barring any objection, I will commit the patch that I posted upthread. Pushed! I'm waiting for other two patches to be reviewed :) Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
(I finally get to catch up here..) At Mon, 22 Mar 2021 13:59:47 +0900, Fujii Masao <masao.fujii@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