On Wed, Dec 09, 2020 at 02:00:44AM +0900, Fujii Masao wrote:
> I agree it might be helpful to display something like
> "checkpointing" or "waiting for checkpoint" in PS title for the
> startup process.
Thanks.
> But, at least for me, it seems strange to display "idle" only for
> checkpointer.
Yeah, I'd rather leave out the part of the patch where we have the
checkpointer say "idle". So I still think that what v3 did upthread,
by just resetting the ps display in all cases, feels more natural. So
we should remove the parts of v5 from checkpointer.c.
+ * Reset the ps status display. We set the status to "idle" for the
+ * checkpointer process, and we clear it for anything else that runs this
+ * code.
+ */
+ if (MyBackendType == B_CHECKPOINTER)
+ set_ps_display("idle");
+ else
+ set_ps_display("");
Regarding this part, this just needs a reset with an empty string.
The second sentence is confusing (it partially comes fronm v3, from
me..). Do we lose anything by removing the second sentence of this
comment?
+ snprintf(activitymsg, sizeof(activitymsg), "creating %s%scheckpoint",
[...]
+ snprintf(activitymsg, sizeof(activitymsg), "creating %srestartpoint",
FWIW, I still fing "running" to sound better than "creating" here. An
extra term I can think of that could be adapted is "performing".
> *If* we want to monitor the current status of
> checkpointer, it should be shown as wait event in pg_stat_activity,
> instead?
That would be WAIT_EVENT_CHECKPOINTER_MAIN, now there has been also on
this thread an argument that you would not have access to
pg_stat_activity until crash recovery completes and consistency is
restored.
--
Michael