Re: shared-memory based stats collector - v70

Hi!

On 09.12.2024 11:03, Bertrand Drouvot wrote:

> There is a missing space. I think that should be " at server..." or "...%llu ".

Thanks for pointing this out. In the other code the elog messages are all on one line,
regardless of their length. Did the same in v3.

On 10.12.2024 09:42, Bertrand Drouvot wrote:
> On Tue, Dec 10, 2024 at 09:54:36AM +0900, Michael Paquier wrote:
>> On Mon, Dec 09, 2024 at 08:03:54AM +0000, Bertrand Drouvot wrote:
>>> Right. OTOH I think that could help the tap test added in da99fedf8c to not
>>> rely on assert enabled build (the tap test could "simply" check for the
>>> WARNING in the logfile instead).
>>
>> That's true. Still, the coverage that we have is also enough for
>> assert builds, which is what the test is going to run with most of the
>> time anyway.
>
> Yeah, that's fine by me and don't see the added value of the WARNING then.

Agreed that this WARNING has no additional value for testing purposes
at pgfarm or ci. Assert is better.
My logic was different.
It's clear that during normal server operation this code should be unreachable.
But we admit that in production deployments it can be executed in case of some bug
that is still unknown to us. Now it is done in such a way that in this case
the server simply skip it and won't notice about it. And no one will know that this happened.
But if there is a warning here, the information will remain in the server logs,
we can find out about it and we can try to reproduce similar behavior
in the testing environment and probably detect a hidden bug like in [1].

Thanks a lot for fixing this!

With the best regards,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[1] https://www.postgresql.org/message-id/56bf8ff9-dd8c-47b2-872a-748ede82af99%40postgrespro.ru