Robert Haas wrote:
> On Mon, Aug 7, 2017 at 8:14 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> 
> wrote:
> > BTW, I noticed that the PG_WAIT_LOCK value that we're using for wait
> > event here (and in the replication slot case) is bogus.  We probably
> > need something new here.
> 
> Yeah, if you're adding a new wait point, you should add document a new
> constant in the appropriate section, probably something under
> PG_WAIT_IPC in this case.

Here's a patch.  It turned to be a bit larger than I initially expected.

Wait events are a maintainability fail IMO.  I think we need to rethink
this stuff; using generated files from a single source containing the C
symbol, text name and doc blurb sounds better.  That can wait for pg11
though.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From e050ef66956935e6dba41fc18e11632ff9f0068f Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Tue, 8 Aug 2017 13:33:47 -0400
Subject: [PATCH] Fix various inadequacies in wait events

In commit 9915de6c1cb2, we introduced a new wait point for replication
slots and incorrectly labelled it as wait event PG_WAIT_LOCK.  That's
wrong, so invent an appropriate new wait event instead, and document it
properly.

While at it, fix numerous other problems in the vicinity:
- two different walreceiver wait events were being mixed up in a single
  wait event; split it out so that they can be distinguished, and
  document the new symbols properly.

- ParallelBitmapPopulate was documented but didn't exist.

- ParallelBitmapScan was not documented

- Logical replication wait events weren't documented

- various symbols had been added in dartboard order in various places.
  Put them back in alphabetical order, as was originally intended.
---
 doc/src/sgml/monitoring.sgml                       | 32 +++++++++++++++++--
 src/backend/postmaster/pgstat.c                    | 36 +++++++++++++---------
 .../libpqwalreceiver/libpqwalreceiver.c            |  4 +--
 src/backend/replication/slot.c                     |  3 +-
 src/include/pgstat.h                               | 16 +++++-----
 5 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index be3dc672bc..eb20c9c543 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1176,6 +1176,14 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting in main loop of checkpointer process.</entry>
         </row>
         <row>
+         <entry><literal>LogicalLauncherMain</></entry>
+         <entry>Waiting in main loop of logical launcher process.</entry>
+        </row>
+        <row>
+         <entry><literal>LogicalApplyMain</></entry>
+         <entry>Waiting in main loop of logical apply process.</entry>
+        </row>
+        <row>
          <entry><literal>PgStatMain</></entry>
          <entry>Waiting in main loop of the statistics collector 
process.</entry>
         </row>
@@ -1213,6 +1221,14 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting to write data from the client.</entry>
         </row>
         <row>
+         <entry><literal>LibPQWalReceiverConnect</></entry>
+         <entry>Waiting in WAL receiver to establish connection to remote 
server.<entry>
+        </row>
+        <row>
+         <entry><literal>LibPQWalReceiverReceive</></entry>
+         <entry>Waiting in WAL receiver to receive data from remote 
server.<entry>
+        </row>
+        <row>
          <entry><literal>SSLOpenServer</></entry>
          <entry>Waiting for SSL while attempting connection.</entry>
         </row>
@@ -1251,6 +1267,14 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting for activity from child process when executing 
<literal>Gather</> node.</entry>
         </row>
         <row>
+         <entry><literal>LogicalSyncData</></entry>
+         <entry>Waiting for logical replication remote server to send data for 
initial table synchronization.</entry>
+        </row>
+        <row>
+         <entry><literal>LogicalSyncStateChange</></entry>
+         <entry>Waiting for logical replication remote server to change 
state.</entry>
+        </row>
+        <row>
          <entry><literal>MessageQueueInternal</></entry>
          <entry>Waiting for other process to be attached in shared message 
queue.</entry>
         </row>
@@ -1271,14 +1295,18 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting for parallel workers to finish computing.</entry>
         </row>
         <row>
-         <entry><literal>ParallelBitmapPopulate</></entry>
-         <entry>Waiting for the leader to populate the TidBitmap.</entry>
+         <entry><literal>ParallelBitmapScan</></entry>
+         <entry>Waiting for parallel bitmap scan to become initialized.</entry>
         </row>
         <row>
          <entry><literal>ProcArrayGroupUpdate</></entry>
          <entry>Waiting for group leader to clear transaction id at 
transaction end.</entry>
         </row>
         <row>
+         <entry><literal>ReplicationSlotDrop</></entry>
+         <entry>Waiting for a replication slot to become inactive to be 
dropped.</entry>
+        </row>
+        <row>
          <entry><literal>SafeSnapshot</></entry>
          <entry>Waiting for a snapshot for a <literal>READ ONLY DEFERRABLE</> 
transaction.</entry>
         </row>
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index a0b0eecbd5..3f5fb796a5 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3481,6 +3481,12 @@ pgstat_get_wait_activity(WaitEventActivity w)
                case WAIT_EVENT_CHECKPOINTER_MAIN:
                        event_name = "CheckpointerMain";
                        break;
+               case WAIT_EVENT_LOGICAL_LAUNCHER_MAIN:
+                       event_name = "LogicalLauncherMain";
+                       break;
+               case WAIT_EVENT_LOGICAL_APPLY_MAIN:
+                       event_name = "LogicalApplyMain";
+                       break;
                case WAIT_EVENT_PGSTAT_MAIN:
                        event_name = "PgStatMain";
                        break;
@@ -3502,12 +3508,6 @@ pgstat_get_wait_activity(WaitEventActivity w)
                case WAIT_EVENT_WAL_WRITER_MAIN:
                        event_name = "WalWriterMain";
                        break;
-               case WAIT_EVENT_LOGICAL_LAUNCHER_MAIN:
-                       event_name = "LogicalLauncherMain";
-                       break;
-               case WAIT_EVENT_LOGICAL_APPLY_MAIN:
-                       event_name = "LogicalApplyMain";
-                       break;
                        /* no default case, so that compiler will warn */
        }
 
@@ -3533,15 +3533,18 @@ pgstat_get_wait_client(WaitEventClient w)
                case WAIT_EVENT_CLIENT_WRITE:
                        event_name = "ClientWrite";
                        break;
+               case WAIT_EVENT_LIBPQWALRECEIVER_CONNECT:
+                       event_name = "LibPQWalReceiverConnect";
+                       break;
+               case WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE:
+                       event_name = "LibPQWalReceiverReceive";
+                       break;
                case WAIT_EVENT_SSL_OPEN_SERVER:
                        event_name = "SSLOpenServer";
                        break;
                case WAIT_EVENT_WAL_RECEIVER_WAIT_START:
                        event_name = "WalReceiverWaitStart";
                        break;
-               case WAIT_EVENT_LIBPQWALRECEIVER:
-                       event_name = "LibPQWalReceiver";
-                       break;
                case WAIT_EVENT_WAL_SENDER_WAIT_WAL:
                        event_name = "WalSenderWaitForWAL";
                        break;
@@ -3579,6 +3582,12 @@ pgstat_get_wait_ipc(WaitEventIPC w)
                case WAIT_EVENT_EXECUTE_GATHER:
                        event_name = "ExecuteGather";
                        break;
+               case WAIT_EVENT_LOGICAL_SYNC_DATA:
+                       event_name = "LogicalSyncData";
+                       break;
+               case WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE:
+                       event_name = "LogicalSyncStateChange";
+                       break;
                case WAIT_EVENT_MQ_INTERNAL:
                        event_name = "MessageQueueInternal";
                        break;
@@ -3600,18 +3609,15 @@ pgstat_get_wait_ipc(WaitEventIPC w)
                case WAIT_EVENT_PROCARRAY_GROUP_UPDATE:
                        event_name = "ProcArrayGroupUpdate";
                        break;
+               case WAIT_EVENT_REPLICATION_SLOT_DROP:
+                       event_name = "ReplicationSlotDrop";
+                       break;
                case WAIT_EVENT_SAFE_SNAPSHOT:
                        event_name = "SafeSnapshot";
                        break;
                case WAIT_EVENT_SYNC_REP:
                        event_name = "SyncRep";
                        break;
-               case WAIT_EVENT_LOGICAL_SYNC_DATA:
-                       event_name = "LogicalSyncData";
-                       break;
-               case WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE:
-                       event_name = "LogicalSyncStateChange";
-                       break;
                        /* no default case, so that compiler will warn */
        }
 
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c 
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 93dd7b5c17..de03362c91 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -181,7 +181,7 @@ libpqrcv_connect(const char *conninfo, bool logical, const 
char *appname,
                                                           WL_LATCH_SET | 
io_flag,
                                                           
PQsocket(conn->streamConn),
                                                           0,
-                                                          
WAIT_EVENT_LIBPQWALRECEIVER);
+                                                          
WAIT_EVENT_LIBPQWALRECEIVER_CONNECT);
 
                /* Emergency bailout? */
                if (rc & WL_POSTMASTER_DEATH)
@@ -582,7 +582,7 @@ libpqrcv_PQexec(PGconn *streamConn, const char *query)
                                                                   WL_LATCH_SET,
                                                                   
PQsocket(streamConn),
                                                                   0,
-                                                                  
WAIT_EVENT_LIBPQWALRECEIVER);
+                                                                  
WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE);
 
                        /* Emergency bailout? */
                        if (rc & WL_POSTMASTER_DEATH)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 08c0b1b285..63e1aaa910 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -391,7 +391,8 @@ retry:
                                                        name, active_pid)));
 
                /* Wait here until we get signaled, and then restart */
-               ConditionVariableSleep(&slot->active_cv, PG_WAIT_LOCK);
+               ConditionVariableSleep(&slot->active_cv,
+                                                          
WAIT_EVENT_REPLICATION_SLOT_DROP);
                ConditionVariableCancelSleep();
                goto retry;
        }
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 6bffe63ad6..43ea55e9eb 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -759,15 +759,15 @@ typedef enum
        WAIT_EVENT_BGWRITER_HIBERNATE,
        WAIT_EVENT_BGWRITER_MAIN,
        WAIT_EVENT_CHECKPOINTER_MAIN,
+       WAIT_EVENT_LOGICAL_LAUNCHER_MAIN,
+       WAIT_EVENT_LOGICAL_APPLY_MAIN,
        WAIT_EVENT_PGSTAT_MAIN,
        WAIT_EVENT_RECOVERY_WAL_ALL,
        WAIT_EVENT_RECOVERY_WAL_STREAM,
        WAIT_EVENT_SYSLOGGER_MAIN,
        WAIT_EVENT_WAL_RECEIVER_MAIN,
        WAIT_EVENT_WAL_SENDER_MAIN,
-       WAIT_EVENT_WAL_WRITER_MAIN,
-       WAIT_EVENT_LOGICAL_LAUNCHER_MAIN,
-       WAIT_EVENT_LOGICAL_APPLY_MAIN
+       WAIT_EVENT_WAL_WRITER_MAIN
 } WaitEventActivity;
 
 /* ----------
@@ -782,9 +782,10 @@ typedef enum
 {
        WAIT_EVENT_CLIENT_READ = PG_WAIT_CLIENT,
        WAIT_EVENT_CLIENT_WRITE,
+       WAIT_EVENT_LIBPQWALRECEIVER_CONNECT,
+       WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE,
        WAIT_EVENT_SSL_OPEN_SERVER,
        WAIT_EVENT_WAL_RECEIVER_WAIT_START,
-       WAIT_EVENT_LIBPQWALRECEIVER,
        WAIT_EVENT_WAL_SENDER_WAIT_WAL,
        WAIT_EVENT_WAL_SENDER_WRITE_DATA
 } WaitEventClient;
@@ -802,6 +803,8 @@ typedef enum
        WAIT_EVENT_BGWORKER_STARTUP,
        WAIT_EVENT_BTREE_PAGE,
        WAIT_EVENT_EXECUTE_GATHER,
+       WAIT_EVENT_LOGICAL_SYNC_DATA,
+       WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE,
        WAIT_EVENT_MQ_INTERNAL,
        WAIT_EVENT_MQ_PUT_MESSAGE,
        WAIT_EVENT_MQ_RECEIVE,
@@ -809,10 +812,9 @@ typedef enum
        WAIT_EVENT_PARALLEL_FINISH,
        WAIT_EVENT_PARALLEL_BITMAP_SCAN,
        WAIT_EVENT_PROCARRAY_GROUP_UPDATE,
+       WAIT_EVENT_REPLICATION_SLOT_DROP,
        WAIT_EVENT_SAFE_SNAPSHOT,
-       WAIT_EVENT_SYNC_REP,
-       WAIT_EVENT_LOGICAL_SYNC_DATA,
-       WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE
+       WAIT_EVENT_SYNC_REP
 } WaitEventIPC;
 
 /* ----------
-- 
2.11.0

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to