On Wed, Feb 17, 2021 at 5:41 AM Peter Geoghegan <p...@bowt.ie> wrote: > > On Tue, Feb 16, 2021 at 11:35 AM Peter Geoghegan <p...@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/
