On Wed, Feb 17, 2021 at 5:41 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> 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.
Yes, you're right. I missed that pages_deleted_not_free is calculated
by (stats->pages_deleted - stats->pages_free) where both are in
IndexBulkDeleteResult.
>
> 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.
Agreed.
>
> (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.)
Agreed.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/