| From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
|---|---|
| To: | Euler Taveira <euler(at)eulerto(dot)com> |
| Cc: | japin <japinli(at)hotmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Subject: | Re: log_min_messages per backend type |
| Date: | 2026-02-06 11:05:27 |
| Message-ID: | 202602061004.yyesvyagjux7@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Here are some fixups for the main patch. I include yours so that CI
will test this one.
* I found some errdetail messages unclear, so I reworded them.
Specifically you had
ERROR: invalid value for parameter "log_min_messages": "backend:error, debug1, backend:warning"
DETAIL: Process type "backend" was already assigned.
which is not completely incorrect but looks weird from the user
perspective. I changed it to
DETAIL: Redundant log level specification for process type "backend".
Also,
ERROR: invalid value for parameter "log_min_messages": "debug1, backend:error, fatal"
DETAIL: Generic log level was already assigned.
I changed to
DETAIL: Redundant specification of default log level.
and added process type in this one:
ERROR: invalid value for parameter "log_min_messages": "backend:error, checkpointer:bar, archiver:debug1"
DETAIL: Unrecognized log level: "bar".
changed to
DETAIL: Unrecognized log level for process type "checkpointer": "bar".
(This one can show an invalid process type if the level is also bogus.
I don't think that's a terrible problem.)
* I'm not fond of the term "generic", so I changed it to "default",
which IMO makes more sense from the user point of view.
* There seemed to be too much guc_mallocking[*] going on; for instance for
ptype. I simplified that by just overwriting the : to a '\0', then we
can use the original string for pg_strcasecmp(). At the bottom of the
loop we restore the ':', so that the code below can reuse the original
values. This is okay because we have already mallocked a copy for
SplitGUCList.
* There's no reason to add a WARNING to every process type in the
proctypelist.h table. We can just set the values to WARNING in the
log_min_messages initializer in guc_tables.c.
* There's no reason AFAICS to have log_min_messages_process_types in
guc_tables.c; nobody else uses it. I made it a local array of char*
inside the check_log_min_messages function.
* There's no need to initialize the newlevel[] array, since we're going
to assign values to every individual element at one point or another.
And for the assigned[] array, we don't need to set the value of each to
'false' in a loop; we can just initialize to '{0}' and the compiler will
do the needful. With this we can remove the loop entirely.
* "guc_free()+list_free()+return false" the whole time in the various
failure places across the loop was tiresome. I put them all in a single
place and jump there with a goto.
* in foreach() loops, tracking of a 'bool first' is unnecessary. You
can just compare foreach_current_index() with 0.
* Rewrote docs and comments
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v10-0001-log_min_messages-per-process-type.patch | text/x-diff | 27.4 KB |
| v10-0002-review.patch | text/x-diff | 17.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Boris Mironov | 2026-02-06 11:08:23 | Re: Idea to enhance pgbench by more modes to generate data (multi-TXNs, UNNEST, COPY BINARY) |
| Previous Message | Michael Paquier | 2026-02-06 10:59:51 | Re: Concerns regarding code in pgstat_backend.c |