Re: please update ps display for recovery checkpoint

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: please update ps display for recovery checkpoint
Date: 2020-12-09 06:15:15
Message-ID: X9Brc31Jj5cemRUw@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message 曾文旌 2020-12-09 06:33:59 Re: [Proposal] Global temporary tables
Previous Message David Rowley 2020-12-09 06:13:48 Re: [PATCH] Keeps tracking the uniqueness with UniqueKey