Re: 64-bit XIDs in deleted nbtree pages

On Sun, Feb 28, 2021 at 8:08 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > Even though "the letter of the law" favors removing the
> > vacuum_cleanup_index_scale_factor GUC + param in the way I have
> > outlined, that is not the only thing that matters -- we must also
> > consider "the spirit of the law".

> > I suppose I could ask Tom what he thinks?
>
> +1

Are you going to start a new thread, or should I?

> Since it seems not a bug I personally think we don't need to do
> anything for back branches. But if we want not to trigger an index
> scan by vacuum_cleanup_index_scale_factor, we could change the default
> value to a high value (say, to 10000) so that it can skip an index
> scan in most cases.

One reason to remove vacuum_cleanup_index_scale_factor in the back
branches is that it removes any need to fix the
"IndexVacuumInfo.num_heap_tuples is inaccurate outside of
btvacuumcleanup-only VACUUMs" bug -- it just won't matter if
btm_last_cleanup_num_heap_tuples is inaccurate anymore. (I am still
not sure about backpatch being a good idea, though.)

> > Another new concern for me (another concern unique to Postgres 13) is
> > autovacuum_vacuum_insert_scale_factor-driven autovacuums.
>
> IIUC the purpose of autovacuum_vacuum_insert_scale_factor is
> visibility map maintenance. And as per this discussion, it seems not
> necessary to do an index scan in btvacuumcleanup() triggered by
> autovacuum_vacuum_insert_scale_factor.

Arguably the question of skipping scanning the index should have been
considered by the autovacuum_vacuum_insert_scale_factor patch when it
was committed for Postgres 13 -- but it wasn't. There is a regression
that was tied to autovacuum_vacuum_insert_scale_factor in Postgres 13
by Mark Callaghan, which I suspect is relevant:

https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html

The blog post says: "Updates - To understand the small regression
mentioned above for the l.i1 test (more CPU & write IO) I repeated the
test with 100M rows using 2 configurations: one disabled index
deduplication and the other disabled insert-triggered autovacuum.
Disabling index deduplication had no effect and disabling
insert-triggered autovacuum resolves the regression."

This is quite specifically with an insert-only workload, with 4
indexes (that's from memory, but I'm pretty sure it's 4). I think that
the failure to account for skipping index scans is probably the big
problem here. Scanning the heap to set VM bits is unlikely to be
expensive compared to the full index scans. An insert-only workload is
going to find most of the heap blocks it scans to set VM bits in
shared_buffers. Not so for the indexes.

So in Postgres 13 we have this autovacuum_vacuum_insert_scale_factor
issue, in addition to the deduplication + btvacuumcleanup issue we
talked about (the problems left by my Postgres 13 bug fix commit
48e12913). These two issues make removing
vacuum_cleanup_index_scale_factor tempting, even in the back branches
-- it might actually be the more conservative approach, at least for
Postgres 13.

--
Peter Geoghegan