On Tue, Feb 25, 2025 at 11:02:40AM -0500, Melanie Plageman wrote:
> This does however leave me with the question of how to handle the
> original question of whether or not to cap the proposed relallfrozen
> to the value of relallvisible when updating stats at the end of
> vacuum. The current code in heap_vacuum_rel() caps relallvisible to
> relpages, so capping relallfrozen to relallvisible would follow that
> pattern. However, the other places relallvisible is updated do no such
> capping (do_analyze_rel(), index_update_stats()). It doesn't seem like
> there is a good reason to do it one place and not the others. So, I
> suggest either removing all the caps and adding a WARNING or capping
> the value in all places. 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. This led me to think that
> a WARNING and no cap would be more effective for heap_vacuum_rel().
I'm currently leaning towards the "remove all caps" idea. But I'm not sure
I totally understand the need for a WARNING. What do we expect users to do
with that information? If it's intended to alert them of possible
corruption, then IMHO a WARNING may be too gentle. I guess we could warn
and suggest a way to fix the value with the new statistics manipulation
functions if it's not that big of a deal, but at that point we might as
well just cap it on our own again. If we don't really expect users to have
to do anything about it, then isn't it just adding unnecessary log noise?
--
nathan
