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 |
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? |