Re: log_min_messages per backend type - Mailing list pgsql-hackers

On 2024-Dec-17, Euler Taveira wrote:

> Sometimes you need to inspect some debug messages from autovacuum worker but

> you cannot apply the same setting for backends (that could rapidly fill the log

> file). This proposal aims to change log_min_messages to have different log

> levels depending on which backend type the message comes from.

> 

> The syntax was changed from enum to string and it accepts a list of elements.

> 

> Instead of enum, it now accepts a comma-separated list of elements (string).

> Each element is LOGLEVEL:BACKENDTYPE.

This format seems unintuitive.  I would have thought you do it the other

way around, "backendtype:loglevel" ... that seems more natural because

it's like assigning the 'loglevel' value to the 'backendtype' element.

  SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1';

I dislike the array of names in variable.c.  We already have an array in

launch_backend.c (child_process_kinds), plus GetBackendTypeDesc in

miscinit.c.  Maybe not for this patch to clean up though.

I think it should be acceptable to configure one global setting with

exceptions for particular backend types:

log_min_messages = WARNING, autovacuum:DEBUG1

Right now I think the code only accepts the unadorned log level if there

are no other items in the list.  I think the proposal downthread is to

use the keyword ALL for this,

  log_min_messages = all:WARNING, autovacuum:DEBUG1   # I don't like this

but I think it's inferior, because then "all" is not really "all", and I

think it would be different if I had said

  log_min_messages = autovacuum:DEBUG1, all:WARNING   # I don't like this

because it looks like the "all" entry should override the one I set for

autovacuum before, which frankly would not make sense to me.

So I think these two lines,

log_min_messages = WARNING, autovacuum:DEBUG1

log_min_messages = autovacuum:DEBUG1, WARNING

should behave identically and mean "set the level for autovacuum to

DEBUG1, and to any other backend type to WARNING.

Also, I think it'd be better to reject duplicates in the list.  Right

now it looks like the last entry for one backend type overrides prior

ones.  I mean

log_min_messages = autovacuum:DEBUG1, autovacuum:ERROR

would set autovacuum to error, but that might be mistake prone if your

string is long.  So implementation-wise I suggest to initialize the

whole newlogminmsgs array to -1, then scan the list of entries (saving

an entry without backend type as the one to use later and) setting every

backend type to the number specified; if we see trying to set a value

that's already different from -1, throw error.  After scanning the whole

log_min_messages array, we scan the newlogminmsgs and set any entries

that are still -1 to the value that we saved before.

The new code in variable.c should be before the /* DATESTYLE */ comment

rather than at the end of the file.

You still have many XXX comments.  Also, postgresql.conf should list the

valid values for backendtype, as well as show an example of a valid

setting.  Please don't use ALLCAPS backend types in the docs, this looks

ugly:

> +        Valid <literal>BACKENDTYPE</literal> values are <literal>ARCHIVER</literal>,

> +        <literal>AUTOVACUUM</literal>, <literal>BACKEND</literal>,

> +        <literal>BGWORKER</literal>, <literal>BGWRITER</literal>,

> +        <literal>CHECKPOINTER</literal>, <literal>LOGGER</literal>,

> +        <literal>SLOTSYNCWORKER</literal>, <literal>WALRECEIVER</literal>,

> +        <literal>WALSENDER</literal>, <literal>WALSUMMARIZER</literal>, and

> +        <literal>WALWRITER</literal>.