Re: Type of wait events WalReceiverWaitStart and WalSenderWaitForWAL - Mailing list pgsql-hackers

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