On Wed, Oct 14, 2026 at 02:32:44PM -0500, Sami Imseih wrote: > A test is good here. I don't think we have enough test coverage for > pgstat startup.
FWIW, I am not convinced that the gain in coverage is worth the cycles on this one. This is only one WARNING with many more patterns to check how pgstat_read_statsfile() is robust with changes in shared_preload_libraries and reuses of the same IDs. > However, I think we could keep the proposed test simpler, and flip the > kind from > variable to fixed once, which is enough. Another idea that I may have mentioned on the lists (or, actually, perhaps may not have mentioned at all on the lists yet), would be to have a test suite that writes arbitrary file patterns that we check on restart. The fact that by design pgstat_read_statsfile() discards everything once one entry is inconsistent makes the check of more than one pattern harder to scale as we would need to force multiple startups.. But if we just added a set of SQL functions able to read or edit a stats file and began publishing pgstat_read_statsfile() or an equivalent of it, it would be a different story. Whether it is worth having or not is a different story. > we do not prevent a fixed-numbered kind from defining these callbacks. > I think we should harden that. With it in place, the new check is > only needed for PGSTAT_FILE_ENTRY_HASH, as a name entry for a > fixed-numbered kind will then always fail the !from_serialized_name > check when PGSTAT_FILE_ENTRY_NAME. Hmm. Yep, I don't see why we should not do that as well at startup. It's one of these startup hardenings. Saying all that, adding an extra check based on fixed_amount if we are dealing with an ENTRY_HASH is of course fine here. If one has the idea to reuse a kind ID, a crash would be surprising, like any of the other checks of this kind. -- Michael
signature.asc
Description: PGP signature
