On Tue, Feb 16, 2021 at 11:35 AM Peter Geoghegan <pg@bowt.ie> wrote:
> Isn't btvacuumcleanup() (or any other amvacuumcleanup() routine)
> entitled to rely on the bulk delete stats being set in the way I've
> described? I assumed that that was okay in general, but I haven't
> tested parallel VACUUM specifically. Will parallel VACUUM really fail
> to ensure that values in bulk stats fields (like pages_deleted and
> pages_free) get set correctly for amvacuumcleanup() callbacks?
I tested the pages_deleted_not_free stuff with a version of my patch
that consistently calls _bt_update_meta_cleanup_info() during
btvacuumcleanup(), and never during btbulkdelete(). And it works just
fine -- including with parallel VACUUM.
Evidently my understanding of what btvacuumcleanup() (or any other
amvacuumcleanup() routine) can expect from bulk delete stats was
correct. It doesn't matter whether or not parallel VACUUM happens to
be involved -- it works just as well.
This is good news, since of course it means that it's okay to stick to
the simple approach of calculating pages_deleted_not_free. Passing
pages_deleted_not_free (a.k.a. btm_last_cleanup_num_delpages) to
_bt_update_meta_cleanup_info() during btvacuumcleanup() works just as
well when combined with my fix for the the
"IndexVacuumInfo.num_heap_tuples is inaccurate during btbulkdelete()"
bug. That approach to fixing the IndexVacuumInfo.num_heap_tuples bug
creates no new problems for my patch. There is still no need to think
about when or how the relevant bulk delete fields (pages_deleted and
pages_free) were set. And it doesn't matter whether or not parallel
VACUUM is involved.
(Of course it's also true that we can't do that on the backbranches.
Purely because we must worry about btpo.xact/oldestBtpoXact on the
backbranches. We'll probably have to teach the code in released
versions to set btm_oldest_btpo_xact and
btm_last_cleanup_num_heap_tuples in separate calls -- since there is
no easy way to "send" the oldestBtpoXact value determined during a
btbulkdelete() to a later corresponding btvacuumcleanup(). That's a
bit of a kludge, but I'm not worried about it.)
--
Peter Geoghegan