Re: refactor backend type lists

On Mon, Jul 28, 2025, at 6:13 PM, Álvaro Herrera wrote:
> I should have let you know that I spent some time on this today as well
> to avoid duplicating efforts. Here are my patches, incorporating your
> fixup -- I hadn't looked at your 0004 yet, so I wrote it differently,
> passing the BackendType enum directly to LogChildExit (as well as
> HandleChildCrash), so it is the former function that does the call to
> GetBackendTypeDesc() to obtain the string. I think this is better
> because the untranslated part of the sentence is enclosed in quotes,
> which I think is better. However it meant I had to add a bit of a hack
> to cope with the background worker separate bgw_type string.
>
> So I came up with things as attached, incorporating your 0003 fixup.
> What do you think?
>
Passing BackendType to HandleChildCrash() and LogChildExit() is a good idea.
Someone might forget to add GetBackendTypeDesc(B_XXX) in the future.

Talking about the messages, I think we should have a clean message. IMO
"process of type" seems confusing at first glance. My suggestion is to use:

checkpointer process (PID %d) ...

and

background worker process "foo" (PID %d) ...

or a short variant

background worker "foo" (PID %d) ...

You added quotes to say it is an untranslated part of the sentence but the
current message doesn't have quotes. The background worker name (addtype)
should have quotes because it is a different string depending on the module.
However, the proctype is a stable and well known string. Isn't it better to
improve the translator command saying "don't translate the proctype" instead of
adding quotes?

Another suggestion is to rename "addtype" variable. The substring "add" is
ambiguous when you want to meant "additional". Although you replaced "procname"
with "proctype", I think "procname" can be a good fit for this variable.

>
> This is likely not final, because the lines for background workers look
> a bit inconsistent:
>
> 2025-07-28 23:10:02.316 CEST worker_spi dynamic[1876557] FATAL:
> terminating background worker "worker_spi dynamic" due to administrator
> command
> 2025-07-28 23:10:02.317 CEST postmaster[1876543] LOG: "background
> worker" process of type "logical replication launcher" (PID 1876552)
> exited with exit code 1
>

It is not related to this patch but this message is annoying and confusing. I
expect in the future we have a notion of in-core background worker so we can
differentiate exit code and omit this message for logical replication (and any
in-core feature that uses background worker). There is a patch from Thomas that
tries to improve it [1].

>
> (I, for one, would be VERY HAPPY to not have to translate the phrase
> "background worker". There's just no reasonable way.)
>
+1. As I said the process type is a stable and well known string that we don't
expect to change.

[1] https://www.postgresql.org/message-id/CAEepm%3D10MtmKeDc1WxBM0PQM9OgtNy%2BRCeWqz40pZRRS3PNo5Q%40mail.gmail.com

--
Euler Taveira
EDB https://www.enterprisedb.com/