On Tue, Feb 25, 2025 at 4:29 PM Melanie Plageman <melanieplage...@gmail.com> wrote: > > On Tue, Feb 25, 2025 at 11:36 AM Greg Sabino Mullane <htamf...@gmail.com> > wrote: > > > > On Tue, Feb 25, 2025 at 11:03 AM Melanie Plageman > > <melanieplage...@gmail.com> wrote: > >> > >> Because users can now manually update these values in pg_class, there > >> wouldn't be a way to detect the difference > >> between a bogus relallfrozen value due to VM corruption or a bogus value > >> due to manual statistics intervention. > > > > Er..you had me until this. If manual monkeying of the system catalogs leads > > to a "bogus" error that resembles a real one, then sow the wind, and reap > > the whirlwind. I don't think that should be a consideration here. > > Oh, the WARNING would only show up in a case of actual VM corruption. > The WARNING proposed is after calling visibilitymap_count() before > updating pg_class, if the value we get from the VM for relallfrozen > exceeds relallvisible. If you manually change pg_class > relallfrozen/relallvisible to bogus values, you wouldn't get a > warning. I meant there wouldn't be a way to detect the difference when > viewing pg_class between bogus values because of a corrupt VM and > bogus values because of manual updates to pg_class. >
It strikes me as a bit odd to have this extra wording in the pg_class documentation: + Every all-frozen page must also be marked + all-visible in the visibility map, so + <structfield>relallfrozen</structfield> should be less than or equal to + <structfield>relallvisible</structfield>. However, if either field is + updated manually or if the visibility map is corrupted, it is possible + for <structfield>relallfrozen</structfield> to exceed + <structfield>relallvisible</structfield>. For example, we don't document that rellallvisible should never exceed relpages, and we aren't normally in the habit of documenting weird behavior that might happen if people go updating the system catalogs. Maybe it's just me, but when I read this earlier, I thought there might be some intended use case for updating the catalog manually that you had in mind and so the comments were warranted (and indeed, it's part of why I thought the warning would be useful for users). But upon reading the thread more and another pass through your updated patches, this doesn't seem to be the case, and I wonder if this language might be more encouraging of people updating catalogs than we would typically be. Robert Treat https://xzilla.net
