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/allaym4dhw4pm...@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
signature.asc
Description: PGP signature
