On Tue, Dec 09, 2025 at 05:15:45PM -0600, Sami Imseih wrote: > Thanks for explaining. If there is a good use-case to add more detail to > the “end” callback, it’s not very obvious yet. Maybe in the future, there > will be a convincing reason to do so.
The step taken by test_custom_var_stats_file_cleanup() for the STATS_READ case shines for its simplicity. The STATS_DISCARD case is also simple: we know that we want to ditch the stats. Now, it is kind of true that the STATS_WRITE case feels a bit disturbing written this way: we let a module take an action, but we don't actually know the state of the main pgstats file when inside the callback. I mean, you can know how things are going on, but it means that a module can just rely on a check if PGSTAT_STAT_PERMANENT_FILENAME is on disk, but an unlink() could have failed as well. So, yes, I am wondering whether we should do what Chao is suggesting, passing an extra state to the callback to let the module know if we have actually succeeded or failed the operations that have been taken on the main stats file before the callback end_extra_stats is called in the three cases. It does not matter for the STATS_READ case, but it may matter for the STATS_DISCARD or STATS_WRITE case. > When we hit the clean-up code on any “error”, it should be accompanied by > an error log. That is > done in all cases inside pgstat.c, and I expect an extension to log the > error as well. FWIW, I still have the same question as the one posted here about the business in pgstat_initialize(), still present in v6: https://www.postgresql.org/message-id/[email protected] This remains unanswered. -- Michael
signature.asc
Description: PGP signature
