Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: log_min_messages per backend type
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <72e809ff-c842-4f98-93f1-26a3cb3d92d0@app.fastmail.com>
Date: Thu, 6 Nov 2025 22:53:00 +0800
Cc: japin <japinli@hotmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Andres Freund <andres@anarazel.de>,
 pgsql-hackers@lists.postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <8AF30EAA-2E9F-4398-B4C6-24C4480CA869@gmail.com>
References: <202507311651.ahhz57cfk6lo@alvherre.pgsql>
 <ME0P300MB04459538922993E1BD0DC051B627A@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM>
 <24b9bfd6-7fba-4946-9b73-244934b89464@app.fastmail.com>
 <72e809ff-c842-4f98-93f1-26a3cb3d92d0@app.fastmail.com>
To: Euler Taveira <euler@eulerto.com>
Archived-At: 
 <https://www.postgresql.org/message-id/8AF30EAA-2E9F-4398-B4C6-24C4480CA869%40gmail.com>
Precedence: bulk

Hi Euler,

I just reviewed the patch and got a few comments.

> On Nov 6, 2025, at 21:09, Euler Taveira <euler@eulerto.com> wrote:
>=20
> On Sun, Oct 5, 2025, at 11:18 AM, Euler Taveira wrote:
>> This new patch contains the following changes:
>>=20
>> - patch was rebased
>> - use commit dbf8cfb4f02
>> - use some GUC memory allocation functions
>> - avoid one memory allocation (suggested by Japin Li)
>> - rename backend type: logger -> syslogger
>> - adjust tests to increase test coverage
>> - improve documentation and comments to reflect the current state
>>=20
>=20
> Patch was rebased since commit fce7c73fba4 broke it. No modifications.
>=20
>=20
> --=20
> Euler Taveira
> EDB   =
https://www.enterprisedb.com/<v5-0001-log_min_messages-per-backend-type.pa=
tch>

1 - variable.c
```
+	/* Initialize the array. */
+	memset(newlevel, WARNING, BACKEND_NUM_TYPES * sizeof(int));
```

I think this statement is wrong, because memset() writes bytes but =
integers, so this statement will set very byte to WARNING, which should =
not be the intention. You will need to use a loop to initialize every =
element of newlevel.

2 - variable.c
```
+	/* Parse string into list of identifiers. */
+	if (!SplitGUCList(rawstring, ',', &elemlist))
+	{
+		/* syntax error in list */
+		GUC_check_errdetail("List syntax is invalid.");
+		guc_free(rawstring);
+		list_free(elemlist);
+		return false;
+	}
```

Every element of elemlist points to a position of rawstring, so it=E2=80=99=
s better to list_free(elemlist) first then guc_free(rawstring).

3 - launch_backend.c
```
 static inline bool
 should_output_to_server(int elevel)
 {
-	return is_log_level_output(elevel, log_min_messages);
+	return is_log_level_output(elevel, =
log_min_messages[MyBackendType]);
 }
```

Is it possible that when this function is called, MyBackendType has not =
been initialized? To be safe, maybe we can check if MyBackendType is 0 =
(B_INVALID), then use the generic log_min_message.

4 - config.sgml
```
+        Valid values are a comma-separated list of =
<literal>backendtype:level</literal>
+        and a single <literal>level</literal>. The list allows it to =
use
+        different levels per backend type. Only the single =
<literal>level</literal>
+        is mandatory (order does not matter) and it is assigned to the =
backend
+        types that are not specified in the list.
+        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>ioworker</literal>,
+        <literal>syslogger</literal>, =
<literal>slotsyncworker</literal>,
+        <literal>startup</literal>, <literal>walreceiver</literal>,
+        <literal>walsender</literal>, <literal>walsummarizer</literal>, =
and
+        <literal>walwriter</literal>.
+        Valid <literal>level</literal> values are =
<literal>DEBUG5</literal>,
+        <literal>DEBUG4</literal>, <literal>DEBUG3</literal>, =
<literal>DEBUG2</literal>,
+        <literal>DEBUG1</literal>, <literal>INFO</literal>, =
<literal>NOTICE</literal>,
+        <literal>WARNING</literal>, <literal>ERROR</literal>, =
<literal>LOG</literal>,
+        <literal>FATAL</literal>, and <literal>PANIC</literal>.  Each =
level includes
+        all the levels that follow it.  The later the level, the fewer =
messages are sent
+        to the log.  The default is <literal>WARNING</literal> for all =
backend types.
+        Note that <literal>LOG</literal> has a different rank here than =
in
```

* =E2=80=9CValid values are =E2=80=A6=E2=80=9D, here =E2=80=9Care=E2=80=9D=
 usually mean both are needed, so maybe change =E2=80=9Care=E2=80=9D to =
=E2=80=9Ccan be=E2=80=9D.
* It says =E2=80=9Cthe single level is mandatory=E2=80=9D, then why =
there is still a default value?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/