Re: Trigger more frequent autovacuums of heavy insert tables

On Thu, Feb 20, 2025 at 07:35:32PM -0500, Melanie Plageman wrote:
> Attache v7 doesn't cap the result for manual stats updating done with
> relation_statistics_update().

Unfortunately, this already needs a rebase...

> I did, however, keep the cap for the
> places where vacuum/analyze/create index update the stats. There the
> number for relallfrozen is coming directly from visibilitymap_count(),
> so it should be correct. I could perhaps add an assert instead, but I
> didn't think that really made sense. An assert is meant to help the
> developer and what could the developer do about the visibility map
> being corrupted.

This might just be personal preference, but I think this is exactly the
sort of thing an assertion is meant for. If we expect the value to always
be correct, and it's not, then our assumptions were wrong or someone has
broken something. In both of these cases, I as a developer would really
like to know so that I don't introduce a latent bug. If we want Postgres
to gracefully handle or detect visibility map corruption, then maybe we
should do both or PANIC.

I do see that heap_vacuum_rel() already caps relallvisible to relpages, but
it's not clear to me whether that's something that we expect to regularly
happen in practice or what the adverse effects might be. So perhaps I'm
misunderstanding the scope and severity of bogus results from
visibilitymap_count(). Commit e6858e6, which added this code, doesn't say
anything about safety, but commit 3d351d9 changed the comment in question
to its current wording. After a very quick skim of the latter's thread
[0], I don't see any discussion about this point, either.

>> >> Should we allow manipulating relallfrozen like we do relallvisible? My
>> >> assumption is that would even be required for the ongoing statistics
>> >> import/export work.
>> >
>> > Why would it be required for the statistics import/export work?
>>
>> It's probably not strictly required, but my naive expectation would be that
>> we'd handle relallfrozen just like relallvisible, which appears to be
>> dumped in the latest stats import/export patch. Is there any reason we
>> shouldn't do the same for relallfrozen?
>
> Nope I don't think so, but I also don't know about how people are
> envisioning using a manually updated relallvisible.

That does seem unlikely. I'd expect it to be more useful for porting
statistics over during pg_upgrade.

[0] https://postgr.es/m/flat/F02298E0-6EF4-49A1-BCB6-C484794D9ACC%40thebuild.com

--
nathan