On 2020/02/13 16:30, Michael Paquier wrote:
On Thu, Feb 13, 2020 at 03:35:50PM +0900, Fujii Masao wrote:
I found that the wait events "LogicalRewriteTruncate" and
"GSSOpenServer" are not documented. I'm thinking to add
them into doc separately if ok.

Nice catch.  The ordering of the entries is not respected either for
GSSOpenServer in pgstat.h.  The portion for the code and the docs can
be fixed in back-branches, but not the enum list in WaitEventClient or
we would have an ABI breakage.  But this can be fixed on HEAD.  Can
you take care of it?
Yes. Patch attached.

logical_rewrite_truncate_v1.patch adds the description of
LogicalRewriteTruncate into the doc. This needs to be
back-patched to v10 where commit 249cf070e3 introduced
LogicalRewriteTruncate event.

gss_open_server_v1.patch adds the description of GSSOpenServer
into the doc and update the code in pgstat_get_wait_client().
This needs to be applied in v12 where commit b0b39f72b9 introduced
GSSOpenServer event.

gss_open_server_for_master_v1.patch does not only what the above
patch does but also update wait event enum into alphabetical order.
This needs to be applied in the master.


           <entry><literal>SyncRep</literal></entry>
           <entry>Waiting for confirmation from remote server during synchronous 
replication.</entry>
          </row>
+        <row>
+         <entry><literal>BackupWaitWalArchive</literal></entry>
+         <entry>Waiting for WAL files required for the backup to be successfully 
archived.</entry>
+        </row>

The category IPC is adapted.  You forgot to update the markup morerows
from "36" to "37", causing the table of the wait events to have a
weird format (the bottom should be incorrect).

Fixed. Thanks for the review!


+               pgstat_report_wait_start(WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE);
                while (XLogArchiveIsBusy(lastxlogfilename) ||
                           XLogArchiveIsBusy(histfilename))
                {
@@ -11120,6 +11121,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, 
TimeLineID *stoptli_p)
                                                                 "but the database 
backup will not be usable without all the WAL segments.")));
                        }
                }
+               pgstat_report_wait_end();

Okay, that position is right.

@@ -3848,6 +3848,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
                case WAIT_EVENT_SYNC_REP:
                        event_name = "SyncRep";
                        break;
+               case WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE:
+                       event_name = "BackupWaitWalArchive";
+                       break;
                        /* no default case, so that compiler will warn */
[...]
@@ -853,7 +853,8 @@ typedef enum
        WAIT_EVENT_REPLICATION_ORIGIN_DROP,
        WAIT_EVENT_REPLICATION_SLOT_DROP,
        WAIT_EVENT_SAFE_SNAPSHOT,
-       WAIT_EVENT_SYNC_REP
+       WAIT_EVENT_SYNC_REP,
+       WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE
  } WaitEventIPC;

It would be good to keep entries in alphabetical order in the header,
the code and in the docs (see the effort from 5ef037c), and your patch
is missing that concept for all three places where it matters for this
new event.

Fixed. Patch attached.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 9129f79bbf..027df985ac 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1293,7 +1293,7 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting in main loop of WAL writer process.</entry>
         </row>
         <row>
-         <entry morerows="7"><literal>Client</literal></entry>
+         <entry morerows="8"><literal>Client</literal></entry>
          <entry><literal>ClientRead</literal></entry>
          <entry>Waiting to read data from the client.</entry>
         </row>
@@ -1301,6 +1301,10 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry><literal>ClientWrite</literal></entry>
          <entry>Waiting to write data to the client.</entry>
         </row>
+        <row>
+         <entry><literal>GSSOpenServer</literal></entry>
+         <entry>Waiting to read data from the client while establishing the 
GSSAPI session.</entry>
+        </row>
         <row>
          <entry><literal>LibPQWalReceiverConnect</literal></entry>
          <entry>Waiting in WAL receiver to establish connection to remote 
server.</entry>
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 7169509a79..59dc4f31ab 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3697,6 +3697,9 @@ pgstat_get_wait_client(WaitEventClient w)
                case WAIT_EVENT_CLIENT_WRITE:
                        event_name = "ClientWrite";
                        break;
+               case WAIT_EVENT_GSS_OPEN_SERVER:
+                       event_name = "GSSOpenServer";
+                       break;
                case WAIT_EVENT_LIBPQWALRECEIVER_CONNECT:
                        event_name = "LibPQWalReceiverConnect";
                        break;
@@ -3715,9 +3718,6 @@ pgstat_get_wait_client(WaitEventClient w)
                case WAIT_EVENT_WAL_SENDER_WRITE_DATA:
                        event_name = "WalSenderWriteData";
                        break;
-               case WAIT_EVENT_GSS_OPEN_SERVER:
-                       event_name = "GSSOpenServer";
-                       break;
                        /* no default case, so that compiler will warn */
        }
 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index aecb6013f0..3a65a51696 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -799,13 +799,13 @@ typedef enum
 {
        WAIT_EVENT_CLIENT_READ = PG_WAIT_CLIENT,
        WAIT_EVENT_CLIENT_WRITE,
+       WAIT_EVENT_GSS_OPEN_SERVER,
        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,
-       WAIT_EVENT_GSS_OPEN_SERVER,
 } WaitEventClient;
 
 /* ----------
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 9129f79bbf..027df985ac 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1293,7 +1293,7 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting in main loop of WAL writer process.</entry>
         </row>
         <row>
-         <entry morerows="7"><literal>Client</literal></entry>
+         <entry morerows="8"><literal>Client</literal></entry>
          <entry><literal>ClientRead</literal></entry>
          <entry>Waiting to read data from the client.</entry>
         </row>
@@ -1301,6 +1301,10 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry><literal>ClientWrite</literal></entry>
          <entry>Waiting to write data to the client.</entry>
         </row>
+        <row>
+         <entry><literal>GSSOpenServer</literal></entry>
+         <entry>Waiting to read data from the client while establishing the 
GSSAPI session.</entry>
+        </row>
         <row>
          <entry><literal>LibPQWalReceiverConnect</literal></entry>
          <entry>Waiting in WAL receiver to establish connection to remote 
server.</entry>
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 7169509a79..59dc4f31ab 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3697,6 +3697,9 @@ pgstat_get_wait_client(WaitEventClient w)
                case WAIT_EVENT_CLIENT_WRITE:
                        event_name = "ClientWrite";
                        break;
+               case WAIT_EVENT_GSS_OPEN_SERVER:
+                       event_name = "GSSOpenServer";
+                       break;
                case WAIT_EVENT_LIBPQWALRECEIVER_CONNECT:
                        event_name = "LibPQWalReceiverConnect";
                        break;
@@ -3715,9 +3718,6 @@ pgstat_get_wait_client(WaitEventClient w)
                case WAIT_EVENT_WAL_SENDER_WRITE_DATA:
                        event_name = "WalSenderWriteData";
                        break;
-               case WAIT_EVENT_GSS_OPEN_SERVER:
-                       event_name = "GSSOpenServer";
-                       break;
                        /* no default case, so that compiler will warn */
        }
 
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 9129f79bbf..411076aaf9 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1331,7 +1331,11 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting in an extension.</entry>
         </row>
         <row>
-         <entry morerows="36"><literal>IPC</literal></entry>
+         <entry morerows="37"><literal>IPC</literal></entry>
+         <entry><literal>BackupWaitWalArchive</literal></entry>
+         <entry>Waiting for WAL files required for the backup to be 
successfully archived.</entry>
+        </row>
+        <row>
          <entry><literal>BgWorkerShutdown</literal></entry>
          <entry>Waiting for background worker to shut down.</entry>
         </row>
diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
index 3813eadfb4..d79417b443 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -11095,6 +11095,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, 
TimeLineID *stoptli_p)
                seconds_before_warning = 60;
                waits = 0;
 
+               pgstat_report_wait_start(WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE);
                while (XLogArchiveIsBusy(lastxlogfilename) ||
                           XLogArchiveIsBusy(histfilename))
                {
@@ -11120,6 +11121,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, 
TimeLineID *stoptli_p)
                                                                 "but the 
database backup will not be usable without all the WAL segments.")));
                        }
                }
+               pgstat_report_wait_end();
 
                ereport(NOTICE,
                                (errmsg("all required WAL segments have been 
archived")));
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 7169509a79..0b2c7ee916 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3737,6 +3737,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
 
        switch (w)
        {
+               case WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE:
+                       event_name = "BackupWaitWalArchive";
+                       break;
                case WAIT_EVENT_BGWORKER_SHUTDOWN:
                        event_name = "BgWorkerShutdown";
                        break;
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index aecb6013f0..348dcdc21f 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -817,7 +817,8 @@ typedef enum
  */
 typedef enum
 {
-       WAIT_EVENT_BGWORKER_SHUTDOWN = PG_WAIT_IPC,
+       WAIT_EVENT_BACKUP_WAIT_WAL_ARCHIVE = PG_WAIT_IPC,
+       WAIT_EVENT_BGWORKER_SHUTDOWN,
        WAIT_EVENT_BGWORKER_STARTUP,
        WAIT_EVENT_BTREE_PAGE,
        WAIT_EVENT_CLOG_GROUP_UPDATE,
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 9129f79bbf..0e5a4a0b86 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1493,7 +1493,7 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry>Waiting to apply WAL at recovery because it is delayed.</entry>
         </row>
         <row>
-         <entry morerows="67"><literal>IO</literal></entry>
+         <entry morerows="68"><literal>IO</literal></entry>
          <entry><literal>BufFileRead</literal></entry>
          <entry>Waiting for a read from a buffered file.</entry>
         </row>
@@ -1609,6 +1609,10 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   
11:34   0:00 postgres: ser
          <entry><literal>LogicalRewriteSync</literal></entry>
          <entry>Waiting for logical rewrite mappings to reach stable 
storage.</entry>
         </row>
+        <row>
+         <entry><literal>LogicalRewriteTruncate</literal></entry>
+         <entry>Waiting for truncate of mapping data during a logical 
rewrite.</entry>
+        </row>
         <row>
          <entry><literal>LogicalRewriteWrite</literal></entry>
          <entry>Waiting for a write of logical rewrite mappings.</entry>

Reply via email to