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
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))
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;
 

Reply via email to