Re: refactor backend type lists - Mailing list pgsql-hackers

From Euler Taveira
Subject Re: refactor backend type lists
Date
Msg-id 08352c9a-5c6e-4653-9825-71f325fec947@app.fastmail.com
Whole thread Raw
In response to refactor backend type lists  (Álvaro Herrera <alvherre@kurilemu.de>)
Responses Re: refactor backend type lists
List pgsql-hackers
On Tue, Jul 15, 2025, at 3:30 PM, Álvaro Herrera wrote:
>
> We have lists of backend types scattered through the tree.  I found two
> current ones, and Euler Taveira wants to add a couple more[1].  His
> patch is actually blocked on not adding more, so this seems worth doing.
> Bikeshedding welcome (for a limited time).
>

I was trying to find a general solution for Andres feedback related to backend
types. It seems your proposal is good enough for future development in this
area.

> A couple points.  First, launch_backend.c has its own strings for
> process names, inconsistent with the ones in miscinit.c.  I think we
> should ignore the ones in launch_backend.c, because they're less
> polished and not used for anything interesting, whereas the ones in
> miscinit.c::GetBackendTypeDesc() are -- particularly init_ps_display and
> pg_stat_activity.
>

Agree.

> Second, in discussion [2] leading to commit 18d67a8d7d30 (Nov 2024) it
> was agreed to add support for translating backend type descriptions.
> I'm not really sure that this is useful.  It would be, if set_ps_display
> and pg_stat_activity used translated names, so that you could match what
> log messages say with what the process lists show.  But I think we've
> historically not translated those.  We have a few translatable strings
> as the argument of HandleChildCrash() in postmaster.c, but those are
> using names that are yet a third source of strings; I'm not a fan of
> this.  (For staters, if a translation decided to use non-ascii chars for
> process names, would that work okay in set_ps_display?  I bet it
> wouldn't, because that's using strlen()).  So I would propose to rewind
> a bit here, and remove translation from all those places so that the
> output is consistent (== usable) between log messages and ps/pg_stat_activity.
>

I'm not sure if it is a good idea to have translated backend description. The
init_ps_display() output is certainly used to obtain information (PID?) from a
process list. There is also the option %b from log_line_prefix that is used to
filter log messages per backend type. The same applies to backend_type column
from pg_stat_activity view.

> Third, I didn't do it here, but HandleChildCrash is currently called
> like
>                 HandleChildCrash(pid, exitstatus,
>                                  _("WAL writer process"));
> creating yet another source of strings to describe process types.
>
> I think it would be much better to avoid that by using
>                 HandleChildCrash(pid, exitstatus,
>                                  _(GetBackendTypeDesc(B_WAL_WRITER));
>
> instead.  If we decide to get rid of the translation, then
>                 HandleChildCrash(pid, exitstatus,
>                                  GetBackendTypeDesc(B_WAL_WRITER);
>

GetBackendTypeDesc() is a good use for this case. Looking at the
HandleChildCrash() function, I don't like the translated string (3rd argument)
with the "process" word too.

> This last one would be my preference actually.  Note that we use this
> string in an error that looks like this (LogChildExit):
>
>         /*------
>           translator: %s is a noun phrase describing a child process, such as
>           "server process" */
>                 (errmsg("%s (PID %d) exited with exit code %d",
>                         procname, pid, WEXITSTATUS(exitstatus)),
>
> I would change this to
> errmsg("process %d of type \"%s\" exited with exit code %d",
>        pid, procname, WEXITSTATUS())
> Note that in the original, the process name is translated, and in my
> proposal it wouldn't be.  (This helps match the log message with "ps"
> and pg_stat_activity).
>

+1. It might break translation. Your proposal is much better.

>
> Fourth: patch 0002 is a necessary hack to get the proctype_list.h
> strings be picked up for translation by gettext in makefiles.  It's
> quite ugly!  I'd rather not have it at all.  I have no idea how to do
> this in Meson.
>

We generally don't translated the other PG_XXX lists. I wouldn't do for the
reasons mentioned above. Let's have a stable list of backend types that we can
filter.

I'm attaching complementary patch that implements what you proposed above. I
also suggest that you rename the new file from proctype_list.h to
proctypelist.h. The other similar files don't use underscore.


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

pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: should we have a fast-path planning for OLTP starjoins?
Next
From: Robert Haas
Date:
Subject: Re: Making pgfdw_report_error statically analyzable