On Fri, Sep 05, 2025 at 09:46:55PM +0100, Mikhail Kot wrote:
> Do you want me to update the patch based on your suggestion on fault
> injection, or update the try/catch to the callers as discussed, or
> it's good to be included in Postgres?
I would prefer the approach of letting the callers deal with the error
handling, by making the callers of pgstat_init_entry() be aware of the
problem based on the result returned, as posted at:
https://www.postgresql.org/message-id/aLlAym4DHW4PM8Gg@paquier.xyz
What I do not like in my patch is the change in
pgstat_read_statsfile(). I have wondered about the availability
argument but there's a risk of discarding the stats based on the
memory pressure, which is different from the current pattern where we
rely entirely on the state of the on-disk file for corruption. So it
should be changed to generate an ERROR, with an errdetail() similar to
pgstat_get_entry_ref() but consistent in style to the other messages
in pgstat_read_statsfile().
The injection point business is useful for testing, but I don't see a
point in including something in the patch, because the code changes
influence the test outputs.
A last thing that I was not able to spend much time on is how much it
is possible to mess up with the shared memory state. If it is worse
than I suspected initially, where an OOM in a first session can cause
crashes because of incorrect dshash entries in shmem depending on the
stats kind fetched, a backpatch will be required, indeed. The change
is not really invasive, so that's OK on this side.
If you can send an updated patch version among these lines, that would
be of course helpful for me. I should be able to get back to the
problem on Monday my time.
--
Michael
