Type of wait events WalReceiverWaitStart and WalSenderWaitForWAL
The wait event WalReceiverWaitStart has been categorized in the type Client.
But why? Walreceiver is waiting for startup process to set the lsn and
timeline while it is reporting WalReceiverWaitStart. So its type should be IPC,
instead?
The wait event WalSenderWaitForWAL has also been categorized in the type
Client. While this wait event is being reported, logical replication walsender
is waiting for not only new WAL to be flushed but also the socket to be
readable and writeable (if there is pending data). I guess that this is why
its type is Client. But ISTM walsender is *mainly* waiting for new WAL to be
flushed by other processes during that period, so I think that it's better
to use IPC as the type of the wait event WalSenderWaitForWAL. Thought?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
At Tue, 16 Mar 2021 03:12:54 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
The wait event WalReceiverWaitStart has been categorized in the type
Client.
But why? Walreceiver is waiting for startup process to set the lsn and
timeline while it is reporting WalReceiverWaitStart. So its type
should be IPC,
instead?The wait event WalSenderWaitForWAL has also been categorized in the
type
Client. While this wait event is being reported, logical replication
walsender
is waiting for not only new WAL to be flushed but also the socket to
be
readable and writeable (if there is pending data). I guess that this
is why
its type is Client. But ISTM walsender is *mainly* waiting for new WAL
to be
flushed by other processes during that period, so I think that it's
better
to use IPC as the type of the wait event WalSenderWaitForWAL. Thought?
I agree that it's definitely not a client wait. It would be either
activity or IPC. My reasoning for the latter is it's similar to
WAIT_EVENT_WAL_RECEIVER_MAIN since both are a wait while
WalReceiverMain to continue. With a difference thatin walreceiver
hears where to start in the latter state.
I don't object if it were categorized to IPC, though.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2021/03/16 11:59, Kyotaro Horiguchi wrote:
At Tue, 16 Mar 2021 03:12:54 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
The wait event WalReceiverWaitStart has been categorized in the type
Client.
But why? Walreceiver is waiting for startup process to set the lsn and
timeline while it is reporting WalReceiverWaitStart. So its type
should be IPC,
instead?The wait event WalSenderWaitForWAL has also been categorized in the
type
Client. While this wait event is being reported, logical replication
walsender
is waiting for not only new WAL to be flushed but also the socket to
be
readable and writeable (if there is pending data). I guess that this
is why
its type is Client. But ISTM walsender is *mainly* waiting for new WAL
to be
flushed by other processes during that period, so I think that it's
better
to use IPC as the type of the wait event WalSenderWaitForWAL. Thought?I agree that it's definitely not a client wait. It would be either
activity or IPC. My reasoning for the latter is it's similar to
WAIT_EVENT_WAL_RECEIVER_MAIN since both are a wait while
WalReceiverMain to continue. With a difference thatin walreceiver
hears where to start in the latter state.I don't object if it were categorized to IPC, though.
Ok. And on my further thought;
There are three calls to WalSndWait() in walsender.c as follow.
1. WalSndLoop() calls WalSndWait() with the wait event
"Activity:WalSenderMain". Both physical and logical replication walsenders
use this function.
2. WalSndWriteData() calls WalSndWait() with the wait event
"Client:WalSenderWriteData". Only logical replication walsender uses
this function.
3. WalSndWaitForWal() calls WalSndWait() with the wait event
"Client:WalSenderWaitForWAL". Only logical replication walsender
uses this function.
These three WalSndWait() basically do the same thing, i.e., wait for the latch
set, timeout, postmaster death, the readable and writeable socket. So you
may think that it's strange to categorize them differently. Maybe it's better
to categorize all of them in Actvitiy?
Or it's better to categorize only WalSenderMain in Activity, and the others
in IPC because only WalSenderMain is reported in walsender's main loop.
At least for me the latter is better because the former, i.e., reporting
three different events for walsender's activity in main loop seems a bit strange.
Thought?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
At Tue, 16 Mar 2021 15:42:27 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2021/03/16 11:59, Kyotaro Horiguchi wrote:
At Tue, 16 Mar 2021 03:12:54 +0900, Fujii Masao
<masao.fujii@oss.nttdata.com> wrote inThe wait event WalReceiverWaitStart has been categorized in the type
Client.
But why? Walreceiver is waiting for startup process to set the lsn and
timeline while it is reporting WalReceiverWaitStart. So its type
should be IPC,
instead?The wait event WalSenderWaitForWAL has also been categorized in the
type
Client. While this wait event is being reported, logical replication
walsender
is waiting for not only new WAL to be flushed but also the socket to
be
readable and writeable (if there is pending data). I guess that this
is why
its type is Client. But ISTM walsender is *mainly* waiting for new WAL
to be
flushed by other processes during that period, so I think that it's
better
to use IPC as the type of the wait event WalSenderWaitForWAL. Thought?I agree that it's definitely not a client wait. It would be either
activity or IPC. My reasoning for the latter is it's similar to
WAIT_EVENT_WAL_RECEIVER_MAIN since both are a wait while
WalReceiverMain to continue. With a difference thatin walreceiver
hears where to start in the latter state.
I don't object if it were categorized to IPC, though.Ok. And on my further thought;
There are three calls to WalSndWait() in walsender.c as follow.1. WalSndLoop() calls WalSndWait() with the wait event
"Activity:WalSenderMain". Both physical and logical replication
walsenders
use this function.
2. WalSndWriteData() calls WalSndWait() with the wait event
"Client:WalSenderWriteData". Only logical replication walsender uses
this function.
3. WalSndWaitForWal() calls WalSndWait() with the wait event
"Client:WalSenderWaitForWAL". Only logical replication walsender
uses this function.These three WalSndWait() basically do the same thing, i.e., wait for
the latch
set, timeout, postmaster death, the readable and writeable socket. So
you
may think that it's strange to categorize them differently. Maybe it's
better
to categorize all of them in Actvitiy?
I think it'd be better that they are categorized by what it is waiting
for.
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.
So whether the three fall into the same category or not doesn't matter
to me.
WAIT_EVENT_WAL_RECEIVER_MAIN(WalReceiverMain) is waiting for new data
to arrive. This looks like an activity to me.
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.)
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.
2 (WAIT_EVENT_WAL_SENDER_WRITE_DATA, currently a wait_client) is the
same to 1.
3 (WAIT_EVENT_WAL_SENDER_WAIT_WAL, currently a wait_client) is the
same to 1.
As the result I'd prefer to categorize all of them to Activity.
Or it's better to categorize only WalSenderMain in Activity, and the
others
in IPC because only WalSenderMain is reported in walsender's main
loop.
I don't think 1, 2 and 3 are Activities. And Activity necessariry
means main loop as I descrived. And as I said,
WAIT_EVENT_WAL_RECEIVER_WAIT_START seems in between IPC and activity
so I don't object to categorized it to IPC.
At least for me the latter is better because the former, i.e.,
reporting
three different events for walsender's activity in main loop seems a
bit strange.
Thought?
Maybe it's the difference in what one consider as the same event.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Wed, 17 Mar 2021 15:31:37 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
WAIT_EVENT_WAL_RECEIVER_MAIN(WalReceiverMain) is waiting for new data
to arrive. This looks like an activity to me.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.)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.2 (WAIT_EVENT_WAL_SENDER_WRITE_DATA, currently a wait_client) is the
same to 1.3 (WAIT_EVENT_WAL_SENDER_WAIT_WAL, currently a wait_client) is the
same to 1.
- As the result I'd prefer to categorize all of them to Activity.
Year, I don't understand what I meant:(
+ As the result I'd prefer to categorize the first two to Activity, and
+ the last three to wait_client.regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
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
Attachments:
0001-Change-the-type-of-WalReceiverWaitStart-wait-event-f.patchtext/plain; charset=UTF-8; name=0001-Change-the-type-of-WalReceiverWaitStart-wait-event-f.patch; x-mac-creator=0; x-mac-type=0Download
From f5a8c8866fc8c750181109a7b3aae9b90e795668 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Fri, 19 Mar 2021 13:56:27 +0900
Subject: [PATCH] Change the type of WalReceiverWaitStart wait event from
Client to IPC.
Previously the type of this wait event was Client. But while this
wait event is being reported, walreceiver process is waiting for
the startup process to set initial data for streaming replication.
It's not waiting for any activity on a socket connected to a user
application or walsender. So this commit changes the type for
WalReceiverWaitStart wait event to IPC.
Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/cdacc27c-37ff-f1a4-20e2-ce19933abfcc@oss.nttdata.com
---
doc/src/sgml/monitoring.sgml | 10 +++++-----
src/backend/postmaster/pgstat.c | 6 +++---
src/include/pgstat.h | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index db4b4e460c..19540206f9 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1171,11 +1171,6 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry><literal>SSLOpenServer</literal></entry>
<entry>Waiting for SSL while attempting connection.</entry>
</row>
- <row>
- <entry><literal>WalReceiverWaitStart</literal></entry>
- <entry>Waiting for startup process to send initial data for streaming
- replication.</entry>
- </row>
<row>
<entry><literal>WalSenderWaitForWAL</literal></entry>
<entry>Waiting for WAL to be flushed in WAL sender process.</entry>
@@ -1771,6 +1766,11 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry><literal>WalrcvExit</literal></entry>
<entry>Waiting for the walreceiver to exit.</entry>
</row>
+ <row>
+ <entry><literal>WalReceiverWaitStart</literal></entry>
+ <entry>Waiting for startup process to send initial data for streaming
+ replication.</entry>
+ </row>
<row>
<entry><literal>XactGroupUpdate</literal></entry>
<entry>Waiting for the group leader to update transaction status at
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 208a33692f..b7af7c2707 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3970,9 +3970,6 @@ pgstat_get_wait_client(WaitEventClient w)
case WAIT_EVENT_SSL_OPEN_SERVER:
event_name = "SSLOpenServer";
break;
- case WAIT_EVENT_WAL_RECEIVER_WAIT_START:
- event_name = "WalReceiverWaitStart";
- break;
case WAIT_EVENT_WAL_SENDER_WAIT_WAL:
event_name = "WalSenderWaitForWAL";
break;
@@ -4127,6 +4124,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
case WAIT_EVENT_WALRCV_EXIT:
event_name = "WalrcvExit";
break;
+ case WAIT_EVENT_WAL_RECEIVER_WAIT_START:
+ event_name = "WalReceiverWaitStart";
+ break;
case WAIT_EVENT_XACT_GROUP_UPDATE:
event_name = "XactGroupUpdate";
break;
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index be43c04802..2c82313550 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -953,7 +953,6 @@ typedef enum
WAIT_EVENT_LIBPQWALRECEIVER_CONNECT,
WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE,
WAIT_EVENT_SSL_OPEN_SERVER,
- WAIT_EVENT_WAL_RECEIVER_WAIT_START,
WAIT_EVENT_WAL_SENDER_WAIT_WAL,
WAIT_EVENT_WAL_SENDER_WRITE_DATA,
} WaitEventClient;
@@ -1010,6 +1009,7 @@ typedef enum
WAIT_EVENT_SAFE_SNAPSHOT,
WAIT_EVENT_SYNC_REP,
WAIT_EVENT_WALRCV_EXIT,
+ WAIT_EVENT_WAL_RECEIVER_WAIT_START,
WAIT_EVENT_XACT_GROUP_UPDATE
} WaitEventIPC;
--
2.27.0
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
Attachments:
WalSenderWriteData.patchtext/plain; charset=UTF-8; name=WalSenderWriteData.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 19540206f9..03f440aa25 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1177,8 +1177,8 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</row>
<row>
<entry><literal>WalSenderWriteData</literal></entry>
- <entry>Waiting for any activity when processing replies from WAL
- receiver in WAL sender process.</entry>
+ <entry>Waiting to write WAL data to WAL receiver in
+ WAL sender process.</entry>
</row>
</tbody>
</tgroup>
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 23baa4498a..acaec753c1 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3115,15 +3115,22 @@ WalSndWakeup(void)
}
/*
- * Wait for readiness on the FeBe socket, or a timeout. The mask should be
- * composed of optional WL_SOCKET_WRITEABLE and WL_SOCKET_READABLE flags. Exit
- * on postmaster death.
+ * Wait for readiness on the FeBe socket, a timeout, or the latch to be set.
+ * The mask should be composed of optional WL_SOCKET_WRITEABLE and
+ * WL_SOCKET_READABLE flags. Exit on postmaster death.
+ *
+ * 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.
*/
static void
WalSndWait(uint32 socket_events, long timeout, uint32 wait_event)
{
WaitEvent event;
+ if (socket_events & WL_SOCKET_WRITEABLE)
+ wait_event = WAIT_EVENT_WAL_SENDER_WRITE_DATA;
+
ModifyWaitEvent(FeBeWaitSet, FeBeWaitSetSocketPos, socket_events, NULL);
if (WaitEventSetWait(FeBeWaitSet, timeout, &event, 1, wait_event) == 1 &&
(event.events & WL_POSTMASTER_DEATH))
WalSenderWaitForWAL.patchtext/plain; charset=UTF-8; name=WalSenderWaitForWAL.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 03f440aa25..02f3aa29db 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1105,7 +1105,13 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</row>
<row>
<entry><literal>WalSenderMain</literal></entry>
- <entry>Waiting in main loop of WAL sender process.</entry>
+ <entry>Waiting in main loop of (physical replication) WAL sender
+ process.</entry>
+ </row>
+ <row>
+ <entry><literal>WalSenderWaitForWAL</literal></entry>
+ <entry>Waiting for WAL to be flushed in main loop of logical
+ replication WAL sender process.</entry>
</row>
<row>
<entry><literal>WalWriterMain</literal></entry>
@@ -1171,10 +1177,6 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry><literal>SSLOpenServer</literal></entry>
<entry>Waiting for SSL while attempting connection.</entry>
</row>
- <row>
- <entry><literal>WalSenderWaitForWAL</literal></entry>
- <entry>Waiting for WAL to be flushed in WAL sender process.</entry>
- </row>
<row>
<entry><literal>WalSenderWriteData</literal></entry>
<entry>Waiting to write WAL data to WAL receiver in
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index b7af7c2707..5ab2cfc852 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3930,6 +3930,9 @@ pgstat_get_wait_activity(WaitEventActivity w)
case WAIT_EVENT_WAL_SENDER_MAIN:
event_name = "WalSenderMain";
break;
+ case WAIT_EVENT_WAL_SENDER_WAIT_WAL:
+ event_name = "WalSenderWaitForWAL";
+ break;
case WAIT_EVENT_WAL_WRITER_MAIN:
event_name = "WalWriterMain";
break;
@@ -3970,9 +3973,6 @@ pgstat_get_wait_client(WaitEventClient w)
case WAIT_EVENT_SSL_OPEN_SERVER:
event_name = "SSLOpenServer";
break;
- case WAIT_EVENT_WAL_SENDER_WAIT_WAL:
- event_name = "WalSenderWaitForWAL";
- break;
case WAIT_EVENT_WAL_SENDER_WRITE_DATA:
event_name = "WalSenderWriteData";
break;
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2c82313550..14913d7c5b 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -934,6 +934,7 @@ typedef enum
WAIT_EVENT_SYSLOGGER_MAIN,
WAIT_EVENT_WAL_RECEIVER_MAIN,
WAIT_EVENT_WAL_SENDER_MAIN,
+ WAIT_EVENT_WAL_SENDER_WAIT_WAL,
WAIT_EVENT_WAL_WRITER_MAIN
} WaitEventActivity;
@@ -953,7 +954,6 @@ typedef enum
WAIT_EVENT_LIBPQWALRECEIVER_CONNECT,
WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE,
WAIT_EVENT_SSL_OPEN_SERVER,
- WAIT_EVENT_WAL_SENDER_WAIT_WAL,
WAIT_EVENT_WAL_SENDER_WRITE_DATA,
} WaitEventClient;
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