MIME-Version: 1.0
References: 
 <CAJDiXgi=Vcp1nh2UoZ5=7BCoR+uXoYzqMsfOfga-XxSUHjFj5Q@mail.gmail.com>
 <eok6kgntbec2vo35xp4eky5ef52ttb7trislyev63ty3ltu533@je4bpifl6spb>
 <CAJDiXgjPGu_cW9nsvciqdgAkqY5o5ugaxDvo3S-rOcqBPcHCPw@mail.gmail.com>
 <CAApHDvqA26O3mnRg=WgASdPrur7bybvGvLesHvnZ0cKQwoEuxA@mail.gmail.com>
In-Reply-To: 
 <CAApHDvqA26O3mnRg=WgASdPrur7bybvGvLesHvnZ0cKQwoEuxA@mail.gmail.com>
From: Daniil Davydov <3danissimo@gmail.com>
Date: Tue, 31 Mar 2026 20:09:56 +0700
Message-ID: 
 <CAJDiXggJkpxM_uqzwXj=c=q3w7JtShx8K24yeZqbkr_FHmVtUA@mail.gmail.com>
Subject: Re: Get rid of redundant StringInfo accumulation
To: David Rowley <dgrowleyml@gmail.com>
Cc: Andres Freund <andres@anarazel.de>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJDiXggJkpxM_uqzwXj%3Dc%3Dq3w7JtShx8K24yeZqbkr_FHmVtUA%40mail.gmail.com>
Precedence: bulk

Hi,

On Tue, Mar 31, 2026 at 6:53=E2=80=AFAM David Rowley <dgrowleyml@gmail.com>=
 wrote:
>
> Looking at the other users of message_level_is_interesting(), every
> one of them that's passing a const is using one of the DEBUG levels,
> in which case the vast majority of the time we will take the
> short-circuit path. All of the direct cases you're adding pass LOG.
> Indirectly, I see some DEBUG3s being passed via
> KnownAssignedXidsDisplay(), so maybe that's worth it. I don't know.

OK, I see that you doubt that users will not set their min_messages above t=
he
"LOG" log level. I asked a few people from my company who are well aware of
typical customer settings. They say that log_min_messages is set to WARNING
the vast majority of the time (hah, I like the phrase, as you can see).
With the client_min_messages parameter the situation is less unambiguous. I=
t
is usually higher than NOTICE, but if postgres starts behaving "suspiciousl=
y",
users immediately lower the parameter value (up to DEBUG5). Pretty obvious =
use
case. Nobody wants to have huge log files, especially if we remember that t=
he
most common error in the production-level systems is "no space left on devi=
ce".

> For me, when I see that, I just assume the person wrote
> the patch because they can, not because they thought it was a good
> idea. Since this place is rife with people using things like static
> analysers and AI tools to give them patch ideas...

Wow. It sounds pretty unpleasant, but it's your right to write so.
Anyway, let's concentrate on the patch.

> very much not inspiring to commit any patch that comes with no proof
> that it does what it's meant to do, especially one that has a
> trade-off that is disadvantageous to people running the standard
> logging levels.

As I wrote above, the log level is typically high in the production-level
systems. Of course, I won't be able to provide evidence because these syste=
ms
are "secured". If you cannot agree with me here, I'm afraid we won't be abl=
e
to agree on most of the fixes in my patch. I am looking forward to getting
your feedback on this issue.

>
> If you still think this is a good idea, then you're going to need to
> show benchmark results and scripts to prove that the extra overhead of
> message_level_is_interesting() is low enough and the advantages of
> skipping the extra StringInfo code is good enough to warrant doing
> this.

Most of the fixes cannot be noticeable in the flamegraphs. If the client us=
es
only WARNING log level in the production then the usefulness of the patch i=
s
obvious for me. So, I think that at first we should consolidate on the issu=
e
above.

--
Best regards,
Daniil Davydov