On 12/8/20, 10:16 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> 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.
That seems fine to me. I think it is most important that the end-of-
recovery and shutdown checkpoints are shown. I'm not sure there's
much value in updating the process title for automatic checkpoints and
checkpoints triggered via the CHECKPOINT command, so IMO we could skip
those, too. I made these changes in the new version of the patch.
> + * 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?
I've fixed this in the new version of the patch.
> + 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".
I think I prefer "performing" over "running" because that's what the
docs use [0]. I've changed it to "performing" in the new version of
the patch.
Nathan
[0] https://www.postgresql.org/docs/devel/wal-configuration.html