Re: Get rid of redundant StringInfo accumulation

Hi,

On Tue, Mar 31, 2026 at 6:53 AM David Rowley <dgrowleyml(at)gmail(dot)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 the
"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. It
is usually higher than NOTICE, but if postgres starts behaving "suspiciously",
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 the
most common error in the production-level systems is "no space left on device".

> 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 systems
are "secured". If you cannot agree with me here, I'm afraid we won't be able
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 uses
only WARNING log level in the production then the usefulness of the patch is
obvious for me. So, I think that at first we should consolidate on the issue
above.

--
Best regards,
Daniil Davydov