On Wed, Jan 18, 2023 at 2:37 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Maybe you're right to be concerned to the degree that you're concerned
> -- I'm not sure. I'm just adding what I see as important context.
The problems in this area tend to be that vac_estimate_reltuples()
behaves as if it sees a random sample, when in fact it's far from
random -- it's the same scanned_pages as last time, and the ten other
times before that. That's a feature of your test case, and other
similar vac_estimate_reltuples test cases that I came up with in the
past. Both scenarios involved skipping using the visibility map in
multiple successive VACUUM operations.
Perhaps we should make vac_estimate_reltuples focus on the pages that
VACUUM newly set all-visible each time (not including all-visible
pages that got scanned despite being all-visible) -- only that subset
of scanned_pages seems to be truly relevant. That way you wouldn't be
able to do stuff like this. We'd have to start explicitly tracking the
number of pages that were newly set in the VM in vacuumlazy.c to be
able to do that, but that seems like a good idea anyway.
This probably has consequences elsewhere, but maybe that's okay. We
know when the existing pg_class has no information, since that is
explicitly encoded by a reltuples of -1. Obviously we'd need to be
careful about stuff like that. Overall, the danger from assuming that
"unsettled" pages (pages that couldn't be newly set all-visible by
VACUUM) have a generic tuple density seems less than the danger of
assuming that they're representative. We know that we're bound to scan
these same pages in the next VACUUM anyway, so they'll have another
chance to impact our view of the table's tuple density at that time.
--
Peter Geoghegan