Re: shared-memory based stats collector - v70

Hi!

On 04.12.2024 18:24, Bertrand Drouvot wrote:
> Thanks! I've the feeling that something has to be fixed, see my comments in
> [1]. It might be that the failed assertion does not handle a "valid" scenario.
>
> [1]: https://www.postgresql.org/message-id/Z1BzI/eMTCOKA%2Bj6%40ip-10-97-1-34.eu-west-3.compute.internal

On 05.12.2024 08:43, Michael Paquier wrote:
> It's really a case that should never be reached because it points to
> an inconsistency in the interactions between the local entry cache in
> a process and the central dshash it attempts to point to, so I don't
> think that there is anything to change here. As Andres has mentioned,
> it has a lot of value by acting as a safety guard in assert builds
> without being annoying for production deployments.

Thanks a lot for for the detailed clarification!
Everything here became clear for me.

On 05.12.2024 11:13, Michael Paquier wrote:
> On Thu, Dec 05, 2024 at 07:37:27AM +0000, Bertrand Drouvot wrote:
>> That said, I think that's worth to update the comment a bit (like in the
>> attached?) as I think that answers a legitimate question someone could have while
>> reading this code.
>
> Perhaps this should provide some details, like the fact that we don't
> expect the server to still have references to entries that are dropped
> at shutdown when writing the stats file as all the backends and/or
> auxiliary processes should have done this cleanup before they are
> gone.

Completely agree that the original comment needs to be revised,
since it implies that it is normal for deleted entries to be here,
but it is not the case.

On 05.12.2024 17:13, Bertrand Drouvot wrote:
> Okay, attached a more elaborated comment.

Looks good for me. Detailed and clear.
Will help to avoid unnecessary questions when reading this code.

Maybe it's worth adding a warning as well,
similar to the one a few lines below in the code?

Like in the patch attached?

With the best regards,

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