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: