Re: refactor backend type lists

From: "Euler Taveira" <euler(at)eulerto(dot)com>
To: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, "Pg Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: refactor backend type lists
Date: 2025-07-28 19:53:26
Message-ID: 08352c9a-5c6e-4653-9825-71f325fec947@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
0003-fixup-Create-a-separate-file-listing-backend-types.patch.nocfbot application/octet-stream 2.9 KB
0004-Adjust-process-type.patch.nocfbot application/octet-stream 6.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2025-07-28 20:01:51 Re: Making pgfdw_report_error statically analyzable
Previous Message Tomas Vondra 2025-07-28 19:44:11 Re: should we have a fast-path planning for OLTP starjoins?