On Fri, Dec 11, 2020 at 06:54:42PM +0000, Bossart, Nathan wrote: > This approach seems reasonable to me. I've attached my take on it.
+ /* Reset the process title */
+ set_ps_display("");
I would still recommend to avoid calling set_ps_display() if there is
no need to so as we avoid useless system calls, so I think that this
stuff had better use a common path for the set and reset logic.
My counter-proposal is like the attached, with the set/reset part not
reversed this time, and the code indented :p
--
Michael
From cb7c41b70ff798870f2eddcaa2782bd47b5fd57f Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Sat, 12 Dec 2020 08:51:48 +0900
Subject: [PATCH v9] Add checkpoint/restartpoint status to ps display.
---
src/backend/access/transam/xlog.c | 45 +++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7e81ce4f17..8dd225c2e1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8687,6 +8687,39 @@ UpdateCheckPointDistanceEstimate(uint64 nbytes)
(0.90 * CheckPointDistanceEstimate + 0.10 * (double) nbytes);
}
+/*
+ * Update the ps display for a process running a checkpoint. Note that
+ * this routine should not do any allocations so as it can be called
+ * from a critical section.
+ */
+static void
+update_checkpoint_display(int flags, bool restartpoint, bool reset)
+{
+ /*
+ * The status is reported only for end-of-recovery and shutdown
+ * checkpoints or shutdown restartpoints. Updating the ps display is
+ * useful in those situations as it may not be possible to rely on
+ * pg_stat_activity to see the status of the checkpointer or the startup
+ * process.
+ */
+ if ((flags & (CHECKPOINT_END_OF_RECOVERY | CHECKPOINT_IS_SHUTDOWN)) == 0)
+ return;
+
+ if (reset)
+ set_ps_display("");
+ else
+ {
+ char activitymsg[128];
+
+ snprintf(activitymsg, sizeof(activitymsg), "performing %s%s%s",
+ (flags & CHECKPOINT_END_OF_RECOVERY) ? "end-of-recovery " : "",
+ (flags & CHECKPOINT_IS_SHUTDOWN) ? "shutdown " : "",
+ restartpoint ? "restartpoint" : "checkpoint");
+ set_ps_display(activitymsg);
+ }
+}
+
+
/*
* Perform a checkpoint --- either during shutdown, or on-the-fly
*
@@ -8905,6 +8938,9 @@ CreateCheckPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, false);
+ /* Update the process title */
+ update_checkpoint_display(flags, false, false);
+
TRACE_POSTGRESQL_CHECKPOINT_START(flags);
/*
@@ -9120,6 +9156,9 @@ CreateCheckPoint(int flags)
/* Real work is done, but log and update stats before releasing lock. */
LogCheckpointEnd(false);
+ /* Reset the process title */
+ update_checkpoint_display(flags, false, true);
+
TRACE_POSTGRESQL_CHECKPOINT_DONE(CheckpointStats.ckpt_bufs_written,
NBuffers,
CheckpointStats.ckpt_segs_added,
@@ -9374,6 +9413,9 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
+ /* Update the process title */
+ update_checkpoint_display(flags, true, false);
+
CheckPointGuts(lastCheckPoint.redo, flags);
/*
@@ -9492,6 +9534,9 @@ CreateRestartPoint(int flags)
/* Real work is done, but log and update before releasing lock. */
LogCheckpointEnd(true);
+ /* Reset the process title */
+ update_checkpoint_display(flags, true, true);
+
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
--
2.29.2
signature.asc
Description: PGP signature
