From 138cbd7810fb4967e480bb49c06524c4e2f18fb5 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sun, 7 Feb 2021 19:24:03 -0800 Subject: [PATCH v3 2/2] Add pages_newly_deleted to VACUUM VERBOSE. pages_newly_deleted reports on the number of pages deleted by the current VACUUM operation. The pages_deleted field continues to report on the total number of deleted pages in the index (as well as pages that are recyclable due to being zeroed in rare cases), without regard to whether or not this VACUUM operation deleted them. --- src/include/access/genam.h | 12 ++++---- src/include/access/nbtree.h | 3 +- src/backend/access/gin/ginvacuum.c | 1 + src/backend/access/gist/gistvacuum.c | 4 ++- src/backend/access/heap/vacuumlazy.c | 6 ++-- src/backend/access/nbtree/nbtpage.c | 43 ++++++++++++++++++--------- src/backend/access/nbtree/nbtree.c | 20 +++++++++---- src/backend/access/spgist/spgvacuum.c | 1 + 8 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/include/access/genam.h b/src/include/access/genam.h index 0eab1508d3..d29c0a8cbb 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -64,10 +64,11 @@ typedef struct IndexVacuumInfo * to communicate additional private data to amvacuumcleanup. * * Note: pages_removed is the amount by which the index physically shrank, - * if any (ie the change in its total size on disk). pages_deleted and - * pages_free refer to free space within the index file. Some index AMs - * may compute num_index_tuples by reference to num_heap_tuples, in which - * case they should copy the estimated_count field from IndexVacuumInfo. + * if any (ie the change in its total size on disk). pages_deleted, + * pages_newly_deleted, and pages_free refer to free space within the index + * file. Some index AMs may compute num_index_tuples by reference to + * num_heap_tuples, in which case they should copy the estimated_count field + * from IndexVacuumInfo. */ typedef struct IndexBulkDeleteResult { @@ -76,7 +77,8 @@ typedef struct IndexBulkDeleteResult bool estimated_count; /* num_index_tuples is an estimate */ double num_index_tuples; /* tuples remaining */ double tuples_removed; /* # removed during vacuum operation */ - BlockNumber pages_deleted; /* # unused pages in index */ + BlockNumber pages_deleted; /* # pages marked deleted (could be by us) */ + BlockNumber pages_newly_deleted; /* # pages marked deleted by us */ BlockNumber pages_free; /* # pages available for reuse */ } IndexBulkDeleteResult; diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index 37d895371a..344fa59092 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -1143,7 +1143,8 @@ extern void _bt_delitems_vacuum(Relation rel, Buffer buf, extern void _bt_delitems_delete_check(Relation rel, Buffer buf, Relation heapRel, TM_IndexDeleteOp *delstate); -extern uint32 _bt_pagedel(Relation rel, Buffer leafbuf); +extern uint32 _bt_pagedel(Relation rel, Buffer leafbuf, + BlockNumber *ndeletedcount); /* * prototypes for functions in nbtsearch.c diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c index 35b85a9bff..7504f57a03 100644 --- a/src/backend/access/gin/ginvacuum.c +++ b/src/backend/access/gin/ginvacuum.c @@ -232,6 +232,7 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn END_CRIT_SECTION(); gvs->result->pages_deleted++; + gvs->result->pages_newly_deleted++; } diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index 94a7e12763..14023e08a6 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -139,6 +139,7 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, stats->estimated_count = false; stats->num_index_tuples = 0; stats->pages_deleted = 0; + stats->pages_newly_deleted = 0; stats->pages_free = 0; /* @@ -281,8 +282,8 @@ restart: { /* Okay to recycle this page */ RecordFreeIndexPage(rel, blkno); - vstate->stats->pages_free++; vstate->stats->pages_deleted++; + vstate->stats->pages_free++; } else if (GistPageIsDeleted(page)) { @@ -640,6 +641,7 @@ gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, MarkBufferDirty(leafBuffer); GistPageSetDeleted(leafPage, txid); stats->pages_deleted++; + stats->pages_newly_deleted++; /* remove the downlink from the parent */ MarkBufferDirty(parentBuffer); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index f3d2265fad..addf243e40 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2521,10 +2521,12 @@ lazy_cleanup_index(Relation indrel, (*stats)->num_index_tuples, (*stats)->num_pages), errdetail("%.0f index row versions were removed.\n" - "%u index pages have been deleted, %u are currently reusable.\n" + "%u index pages have been deleted, %u are newly deleted, %u are currently reusable.\n" "%s.", (*stats)->tuples_removed, - (*stats)->pages_deleted, (*stats)->pages_free, + (*stats)->pages_deleted, + (*stats)->pages_newly_deleted, + (*stats)->pages_free, pg_rusage_show(&ru0)))); } diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c index f748e72539..2e86d64432 100644 --- a/src/backend/access/nbtree/nbtpage.c +++ b/src/backend/access/nbtree/nbtpage.c @@ -50,7 +50,7 @@ static bool _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, static bool _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno, bool *rightsib_empty, - uint32 *ndeleted); + BlockNumber *ndeletedcount); static bool _bt_lock_subtree_parent(Relation rel, BlockNumber child, BTStack stack, Buffer *subtreeparent, @@ -1813,18 +1813,31 @@ _bt_rightsib_halfdeadflag(Relation rel, BlockNumber leafrightsib) * should never pass a buffer containing an existing deleted page here. The * lock and pin on caller's buffer will be dropped before we return. * - * Returns the number of pages successfully deleted (zero if page cannot - * be deleted now; could be more than one if parent or right sibling pages - * were deleted too). Note that this does not include pages that we delete - * that the btvacuumscan scan has yet to reach; they'll get counted later - * instead. + * Returns the number of pages successfully physically deleted (zero if page + * cannot be deleted now; could be more than one if parent or right sibling + * pages were deleted too). Caller uses return value to maintain bulk stats' + * pages_newly_deleted value. + * + * Maintains *ndeletedcount for caller, which is ultimately used as the + * pages_deleted value in bulk delete stats for entire VACUUM. When we're + * called *ndeletedcount is the current total count of pages deleted in the + * index prior to current scanblkno block/position in btvacuumscan. This + * includes existing deleted pages (pages deleted by a previous VACUUM), and + * pages that we delete during current VACUUM. We need to cooperate closely + * with caller here so that whole VACUUM operation reliably avoids any double + * counting of subsidiary-to-leafbuf pages that we delete in passing. If such + * pages happen to be from a block number that is ahead of the current + * scanblkno position, then caller is expected to count them directly later + * on. It's simpler for us to understand caller's requirements than it would + * be for caller to understand when or how a deleted page became deleted after + * the fact. * * NOTE: this leaks memory. Rather than trying to clean up everything * carefully, it's better to run it in a temp context that can be reset * frequently. */ uint32 -_bt_pagedel(Relation rel, Buffer leafbuf) +_bt_pagedel(Relation rel, Buffer leafbuf, BlockNumber *ndeletedcount) { uint32 ndeleted = 0; BlockNumber rightsib; @@ -1834,7 +1847,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) /* * Save original leafbuf block number from caller. Only deleted blocks - * that are <= scanblkno get counted in ndeleted return value. + * that are <= scanblkno are accounted for by *ndeletedcount. */ BlockNumber scanblkno = BufferGetBlockNumber(leafbuf); @@ -2032,7 +2045,7 @@ _bt_pagedel(Relation rel, Buffer leafbuf) { /* Check for interrupts in _bt_unlink_halfdead_page */ if (!_bt_unlink_halfdead_page(rel, leafbuf, scanblkno, - &rightsib_empty, &ndeleted)) + &rightsib_empty, ndeletedcount)) { /* * _bt_unlink_halfdead_page should never fail, since we @@ -2045,6 +2058,8 @@ _bt_pagedel(Relation rel, Buffer leafbuf) Assert(false); return ndeleted; } + ndeleted++; + /* _bt_unlink_halfdead_page probably incremented ndeletedcount */ } Assert(P_ISLEAF(opaque) && P_ISDELETED(opaque) && @@ -2316,7 +2331,7 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack) */ static bool _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno, - bool *rightsib_empty, uint32 *ndeleted) + bool *rightsib_empty, BlockNumber *ndeletedcount) { BlockNumber leafblkno = BufferGetBlockNumber(leafbuf); BlockNumber leafleftsib; @@ -2726,12 +2741,12 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno, _bt_relbuf(rel, buf); /* - * If btvacuumscan won't revisit this page in a future btvacuumpage call - * and count it as deleted then, we count it as deleted by current - * btvacuumpage call + * Maintain *ndeletedcount, per _bt_pagedel() header comments. This is + * how _bt_pagedel() helps the entire VACUUM operation with maintaining + * pages_deleted field from the bulk delete stats. */ if (target <= scanblkno) - (*ndeleted)++; + (*ndeletedcount)++; return true; } diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 12ad877b70..604538297d 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -1001,6 +1001,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, stats->estimated_count = false; stats->num_index_tuples = 0; stats->pages_deleted = 0; + stats->pages_newly_deleted = 0; stats->pages_free = 0; /* Set up info to pass down to btvacuumpage */ @@ -1229,8 +1230,8 @@ backtrack: else if (P_ISHALFDEAD(opaque)) { /* - * Half-dead leaf page. Try to delete now. Might update - * pages_deleted below. + * Half-dead leaf page. Try to delete now. Might end up incrementing + * pages_newly_deleted/pages_deleted below. */ attempt_pagedel = true; } @@ -1442,12 +1443,19 @@ backtrack: oldcontext = MemoryContextSwitchTo(vstate->pagedelcontext); /* - * We trust the _bt_pagedel return value because it does not include - * any page that a future call here from btvacuumscan is expected to - * count. There will be no double-counting. + * _bt_pagedel return value is simply the number of pages directly + * deleted on each call. This is just added to pages_newly_deleted, + * which counts the number of pages marked deleted in current VACUUM. + * + * We need to maintain pages_deleted more carefully here, though. We + * cannot just add the same _bt_pagedel return value to pages_deleted + * because that double-counts pages that are deleted within + * _bt_pagedel that will become scanblkno in a later call here from + * btvacuumscan. */ Assert(blkno == scanblkno); - stats->pages_deleted += _bt_pagedel(rel, buf); + stats->pages_newly_deleted += + _bt_pagedel(rel, buf, &stats->pages_deleted); MemoryContextSwitchTo(oldcontext); /* pagedel released buffer, so we shouldn't */ diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index 0d02a02222..a9ffca5183 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -891,6 +891,7 @@ spgvacuumscan(spgBulkDeleteState *bds) /* Report final stats */ bds->stats->num_pages = num_pages; + bds->stats->pages_newly_deleted = bds->stats->pages_deleted; bds->stats->pages_free = bds->stats->pages_deleted; } -- 2.27.0