Hi,
I've written a patch set for vacuum to use the streaming read interface
proposed in [1]. Making lazy_scan_heap() async-friendly required a bit
of refactoring of lazy_scan_heap() and lazy_scan_skip(). I needed to
confine all of the skipping logic -- previously spread across
lazy_scan_heap() and lazy_scan_skip() -- to lazy_scan_skip(). All of the
patches doing this and other preparation for vacuum to use the streaming
read API can be applied on top of master. The attached patch set does
this.
There are a few comments that still need to be updated. I also noticed I
needed to reorder and combine a couple of the commits. I wanted to
register this for the january commitfest, so I didn't quite have time
for the finishing touches.
- Melanie
[1]
https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com
From 9cd579d6a20aef2aeeab6ef50d72e779d75bf7cd Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 30 Dec 2023 16:18:40 -0500
Subject: [PATCH v1 1/7] lazy_scan_skip remove unnecessary local var rel_pages
lazy_scan_skip() only uses vacrel->rel_pages twice, so there seems to be
no reason to save it in a local variable, rel_pages.
---
src/backend/access/heap/vacuumlazy.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 3b9299b8924..c4e0c077694 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1302,13 +1302,12 @@ static BlockNumber
lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
bool *next_unskippable_allvis, bool *skipping_current_range)
{
- BlockNumber rel_pages = vacrel->rel_pages,
- next_unskippable_block = next_block,
+ BlockNumber next_unskippable_block = next_block,
nskippable_blocks = 0;
bool skipsallvis = false;
*next_unskippable_allvis = true;
- while (next_unskippable_block < rel_pages)
+ while (next_unskippable_block < vacrel->rel_pages)
{
uint8 mapbits = visibilitymap_get_status(vacrel->rel,
next_unskippable_block,
@@ -1331,7 +1330,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
*
* Implement this by always treating the last block as unsafe to skip.
*/
- if (next_unskippable_block == rel_pages - 1)
+ if (next_unskippable_block == vacrel->rel_pages - 1)
break;
/* DISABLE_PAGE_SKIPPING makes all skipping unsafe */
--
2.37.2
From 314dd9038593610583e4fe60ab62e0d46ea3be86 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 30 Dec 2023 16:30:59 -0500
Subject: [PATCH v1 2/7] lazy_scan_skip remove unneeded local var
nskippable_blocks
nskippable_blocks can be easily derived from next_unskippable_block's
progress when compared to the passed in next_block.
---
src/backend/access/heap/vacuumlazy.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c4e0c077694..3b28ea2cdb5 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1302,8 +1302,7 @@ static BlockNumber
lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
bool *next_unskippable_allvis, bool *skipping_current_range)
{
- BlockNumber next_unskippable_block = next_block,
- nskippable_blocks = 0;
+ BlockNumber next_unskippable_block = next_block;
bool skipsallvis = false;
*next_unskippable_allvis = true;
@@ -1360,7 +1359,6 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
vacuum_delay_point();
next_unskippable_block++;
- nskippable_blocks++;
}
/*
@@ -1373,7 +1371,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
* non-aggressive VACUUMs. If the range has any all-visible pages then
* skipping makes updating relfrozenxid unsafe, which is a real downside.
*/
- if (nskippable_blocks < SKIP_PAGES_THRESHOLD)
+ if (next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD)
*skipping_current_range = false;
else
{
--
2.37.2
From 67043818003faa9cf3cdf10e6fdc6cbf6f8eee4c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 30 Dec 2023 16:22:12 -0500
Subject: [PATCH v1 3/7] Add lazy_scan_skip unskippable state
Future commits will remove all skipping logic from lazy_scan_heap() and
confine it to lazy_scan_skip(). To make those commits more clear, first
introduce the struct, VacSkipState, which will maintain the variables
needed to skip ranges less than SKIP_PAGES_THRESHOLD.
While we are at it, add additional information to the lazy_scan_skip()
comment, including descriptions of the role and expectations for its
function parameters.
---
src/backend/access/heap/vacuumlazy.c | 105 ++++++++++++++++-----------
src/tools/pgindent/typedefs.list | 1 +
2 files changed, 64 insertions(+), 42 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 3b28ea2cdb5..6f9c2446c56 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -238,13 +238,24 @@ typedef struct LVSavedErrInfo
VacErrPhase phase;
} LVSavedErrInfo;
+/*
+ * Parameters maintained by lazy_scan_skip() to manage skipping ranges of pages
+ * greater than SKIP_PAGES_THRESHOLD.
+ */
+typedef struct VacSkipState
+{
+ /* Next unskippable block */
+ BlockNumber next_unskippable_block;
+ /* Next unskippable block's visibility status */
+ bool next_unskippable_allvis;
+ /* Whether or not skippable blocks should be skipped */
+ bool skipping_current_range;
+} VacSkipState;
/* non-export function prototypes */
static void lazy_scan_heap(LVRelState *vacrel);
-static BlockNumber lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer,
- BlockNumber next_block,
- bool *next_unskippable_allvis,
- bool *skipping_current_range);
+static void lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
+ BlockNumber next_block, Buffer *vmbuffer);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool sharelock, Buffer vmbuffer);
@@ -826,12 +837,10 @@ lazy_scan_heap(LVRelState *vacrel)
{
BlockNumber rel_pages = vacrel->rel_pages,
blkno,
- next_unskippable_block,
next_fsm_block_to_vacuum = 0;
+ VacSkipState vacskip;
VacDeadItems *dead_items = vacrel->dead_items;
Buffer vmbuffer = InvalidBuffer;
- bool next_unskippable_allvis,
- skipping_current_range;
const int initprog_index[] = {
PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_TOTAL_HEAP_BLKS,
@@ -846,9 +855,7 @@ lazy_scan_heap(LVRelState *vacrel)
pgstat_progress_update_multi_param(3, initprog_index, initprog_val);
/* Set up an initial range of skippable blocks using the visibility map */
- next_unskippable_block = lazy_scan_skip(vacrel, &vmbuffer, 0,
- &next_unskippable_allvis,
- &skipping_current_range);
+ lazy_scan_skip(vacrel, &vacskip, 0, &vmbuffer);
for (blkno = 0; blkno < rel_pages; blkno++)
{
Buffer buf;
@@ -856,26 +863,23 @@ lazy_scan_heap(LVRelState *vacrel)
bool all_visible_according_to_vm;
LVPagePruneState prunestate;
- if (blkno == next_unskippable_block)
+ if (blkno == vacskip.next_unskippable_block)
{
/*
* Can't skip this page safely. Must scan the page. But
* determine the next skippable range after the page first.
*/
- all_visible_according_to_vm = next_unskippable_allvis;
- next_unskippable_block = lazy_scan_skip(vacrel, &vmbuffer,
- blkno + 1,
- &next_unskippable_allvis,
- &skipping_current_range);
+ all_visible_according_to_vm = vacskip.next_unskippable_allvis;
+ lazy_scan_skip(vacrel, &vacskip, blkno + 1, &vmbuffer);
- Assert(next_unskippable_block >= blkno + 1);
+ Assert(vacskip.next_unskippable_block >= blkno + 1);
}
else
{
/* Last page always scanned (may need to set nonempty_pages) */
Assert(blkno < rel_pages - 1);
- if (skipping_current_range)
+ if (vacskip.skipping_current_range)
continue;
/* Current range is too small to skip -- just scan the page */
@@ -1280,15 +1284,34 @@ lazy_scan_heap(LVRelState *vacrel)
* lazy_scan_skip() -- set up range of skippable blocks using visibility map.
*
* lazy_scan_heap() calls here every time it needs to set up a new range of
- * blocks to skip via the visibility map. Caller passes the next block in
- * line. We return a next_unskippable_block for this range. When there are
- * no skippable blocks we just return caller's next_block. The all-visible
- * status of the returned block is set in *next_unskippable_allvis for caller,
- * too. Block usually won't be all-visible (since it's unskippable), but it
- * can be during aggressive VACUUMs (as well as in certain edge cases).
+ * blocks to skip via the visibility map. Caller passes next_block, the next
+ * block in line. The parameters of the skipped range are recorded in vacskip.
+ * vacrel is an in/out parameter here; vacuum options and information about the
+ * relation are read and vacrel->skippedallvis is set to ensure we don't
+ * advance relfrozenxid when we have skipped vacuuming all visible blocks.
+ *
+ * vmbuffer will contain the block from the VM containing visibility
+ * information for the next unskippable heap block. We may end up needed a
+ * different block from the VM (if we decide not to skip a skippable block).
+ * This is okay; visibilitymap_pin() will take care of this while processing
+ * the block.
+ *
+ * A block is unskippable if it is not all visible according to the visibility
+ * map. It is also unskippable if it is the last block in the relation, if the
+ * vacuum is an aggressive vacuum, or if DISABLE_PAGE_SKIPPING was passed to
+ * vacuum.
*
- * Sets *skipping_current_range to indicate if caller should skip this range.
- * Costs and benefits drive our decision. Very small ranges won't be skipped.
+ * Even if a block is skippable, we may choose not to skip it if the range of
+ * skippable blocks is too small (below SKIP_PAGES_THRESHOLD). As a
+ * consequence, we must keep track of the next truly unskippable block and its
+ * visibility status along with whether or not we are skipping the current
+ * range of skippable blocks. This can be used to derive the next block
+ * lazy_scan_heap() must process and its visibility status.
+ *
+ * The block number and visibility status of the next unskippable block are set
+ * in vacskip->next_unskippable_block and next_unskippable_allvis.
+ * vacskip->skipping_current_range indicates to the caller whether or not it is
+ * processing a skippable (and thus all-visible) block.
*
* Note: our opinion of which blocks can be skipped can go stale immediately.
* It's okay if caller "misses" a page whose all-visible or all-frozen marking
@@ -1298,24 +1321,24 @@ lazy_scan_heap(LVRelState *vacrel)
* older XIDs/MXIDs. The vacrel->skippedallvis flag will be set here when the
* choice to skip such a range is actually made, making everything safe.)
*/
-static BlockNumber
-lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
- bool *next_unskippable_allvis, bool *skipping_current_range)
+static void
+lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
+ BlockNumber next_block, Buffer *vmbuffer)
{
- BlockNumber next_unskippable_block = next_block;
bool skipsallvis = false;
- *next_unskippable_allvis = true;
- while (next_unskippable_block < vacrel->rel_pages)
+ vacskip->next_unskippable_block = next_block;
+ vacskip->next_unskippable_allvis = true;
+ while (vacskip->next_unskippable_block < vacrel->rel_pages)
{
uint8 mapbits = visibilitymap_get_status(vacrel->rel,
- next_unskippable_block,
+ vacskip->next_unskippable_block,
vmbuffer);
if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
{
Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0);
- *next_unskippable_allvis = false;
+ vacskip->next_unskippable_allvis = false;
break;
}
@@ -1329,14 +1352,14 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
*
* Implement this by always treating the last block as unsafe to skip.
*/
- if (next_unskippable_block == vacrel->rel_pages - 1)
+ if (vacskip->next_unskippable_block == vacrel->rel_pages - 1)
break;
/* DISABLE_PAGE_SKIPPING makes all skipping unsafe */
if (!vacrel->skipwithvm)
{
/* Caller shouldn't rely on all_visible_according_to_vm */
- *next_unskippable_allvis = false;
+ vacskip->next_unskippable_allvis = false;
break;
}
@@ -1358,7 +1381,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
}
vacuum_delay_point();
- next_unskippable_block++;
+ vacskip->next_unskippable_block++;
}
/*
@@ -1371,16 +1394,14 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
* non-aggressive VACUUMs. If the range has any all-visible pages then
* skipping makes updating relfrozenxid unsafe, which is a real downside.
*/
- if (next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD)
- *skipping_current_range = false;
+ if (vacskip->next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD)
+ vacskip->skipping_current_range = false;
else
{
- *skipping_current_range = true;
+ vacskip->skipping_current_range = true;
if (skipsallvis)
vacrel->skippedallvis = true;
}
-
- return next_unskippable_block;
}
/*
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index e37ef9aa76d..bd008e1699b 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2955,6 +2955,7 @@ VacOptValue
VacuumParams
VacuumRelation
VacuumStmt
+VacSkipState
ValidIOData
ValidateIndexState
ValuesScan
--
2.37.2
From 387db30b7e06f450dc7f60494751f78e00d43272 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 30 Dec 2023 16:59:27 -0500
Subject: [PATCH v1 4/7] Confine vacuum skip logic to lazy_scan_skip
In preparation for vacuum to use the streaming read interface (and eventually
AIO), refactor vacuum's logic for skipping blocks such that it is entirely
confined to lazy_scan_skip(). This turns lazy_scan_skip() and the VacSkipState
it uses into an iterator which yields blocks to lazy_scan_heap(). Such a
structure is conducive to an async interface.
By always calling lazy_scan_skip() -- instead of only when we have reached the
next unskippable block, we no longer need the skipping_current_range variable.
lazy_scan_heap() no longer needs to manage the skipped range -- checking if we
reached the end in order to then call lazy_scan_skip(). And lazy_scan_skip()
can derive the visibility status of a block from whether or not we are in a
skippable range -- that is, whether or not the next_block is equal to the next
unskippable block.
ci-os-only:
---
src/backend/access/heap/vacuumlazy.c | 230 ++++++++++++++-------------
1 file changed, 117 insertions(+), 113 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 6f9c2446c56..5070c3fe744 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -248,14 +248,13 @@ typedef struct VacSkipState
BlockNumber next_unskippable_block;
/* Next unskippable block's visibility status */
bool next_unskippable_allvis;
- /* Whether or not skippable blocks should be skipped */
- bool skipping_current_range;
} VacSkipState;
/* non-export function prototypes */
static void lazy_scan_heap(LVRelState *vacrel);
-static void lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
- BlockNumber next_block, Buffer *vmbuffer);
+static BlockNumber lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
+ BlockNumber blkno, Buffer *vmbuffer,
+ bool *all_visible_according_to_vm);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool sharelock, Buffer vmbuffer);
@@ -836,9 +835,12 @@ static void
lazy_scan_heap(LVRelState *vacrel)
{
BlockNumber rel_pages = vacrel->rel_pages,
- blkno,
next_fsm_block_to_vacuum = 0;
- VacSkipState vacskip;
+ bool all_visible_according_to_vm;
+
+ /* relies on InvalidBlockNumber overflowing to 0 */
+ BlockNumber blkno = InvalidBlockNumber;
+ VacSkipState vacskip = {.next_unskippable_block = InvalidBlockNumber};
VacDeadItems *dead_items = vacrel->dead_items;
Buffer vmbuffer = InvalidBuffer;
const int initprog_index[] = {
@@ -854,37 +856,17 @@ lazy_scan_heap(LVRelState *vacrel)
initprog_val[2] = dead_items->max_items;
pgstat_progress_update_multi_param(3, initprog_index, initprog_val);
- /* Set up an initial range of skippable blocks using the visibility map */
- lazy_scan_skip(vacrel, &vacskip, 0, &vmbuffer);
- for (blkno = 0; blkno < rel_pages; blkno++)
+ while (true)
{
Buffer buf;
Page page;
- bool all_visible_according_to_vm;
LVPagePruneState prunestate;
- if (blkno == vacskip.next_unskippable_block)
- {
- /*
- * Can't skip this page safely. Must scan the page. But
- * determine the next skippable range after the page first.
- */
- all_visible_according_to_vm = vacskip.next_unskippable_allvis;
- lazy_scan_skip(vacrel, &vacskip, blkno + 1, &vmbuffer);
-
- Assert(vacskip.next_unskippable_block >= blkno + 1);
- }
- else
- {
- /* Last page always scanned (may need to set nonempty_pages) */
- Assert(blkno < rel_pages - 1);
-
- if (vacskip.skipping_current_range)
- continue;
+ blkno = lazy_scan_skip(vacrel, &vacskip, blkno + 1,
+ &vmbuffer, &all_visible_according_to_vm);
- /* Current range is too small to skip -- just scan the page */
- all_visible_according_to_vm = true;
- }
+ if (blkno == InvalidBlockNumber)
+ break;
vacrel->scanned_pages++;
@@ -1281,20 +1263,13 @@ lazy_scan_heap(LVRelState *vacrel)
}
/*
- * lazy_scan_skip() -- set up range of skippable blocks using visibility map.
- *
- * lazy_scan_heap() calls here every time it needs to set up a new range of
- * blocks to skip via the visibility map. Caller passes next_block, the next
- * block in line. The parameters of the skipped range are recorded in vacskip.
- * vacrel is an in/out parameter here; vacuum options and information about the
- * relation are read and vacrel->skippedallvis is set to ensure we don't
- * advance relfrozenxid when we have skipped vacuuming all visible blocks.
+ * lazy_scan_skip() -- get next block for vacuum to process
*
- * vmbuffer will contain the block from the VM containing visibility
- * information for the next unskippable heap block. We may end up needed a
- * different block from the VM (if we decide not to skip a skippable block).
- * This is okay; visibilitymap_pin() will take care of this while processing
- * the block.
+ * lazy_scan_heap() calls here every time it needs to get the next block to
+ * prune and vacuum, using the visibility map, vacuum options, and various
+ * thresholds to skip blocks which do not need to be processed. Caller passes
+ * next_block, the next block in line. This block may end up being skipped.
+ * lazy_scan_skip() returns the next block that needs to be processed.
*
* A block is unskippable if it is not all visible according to the visibility
* map. It is also unskippable if it is the last block in the relation, if the
@@ -1304,14 +1279,26 @@ lazy_scan_heap(LVRelState *vacrel)
* Even if a block is skippable, we may choose not to skip it if the range of
* skippable blocks is too small (below SKIP_PAGES_THRESHOLD). As a
* consequence, we must keep track of the next truly unskippable block and its
- * visibility status along with whether or not we are skipping the current
- * range of skippable blocks. This can be used to derive the next block
- * lazy_scan_heap() must process and its visibility status.
+ * visibility status separate from the next block lazy_scan_heap() should
+ * process (and its visibility status).
*
* The block number and visibility status of the next unskippable block are set
- * in vacskip->next_unskippable_block and next_unskippable_allvis.
- * vacskip->skipping_current_range indicates to the caller whether or not it is
- * processing a skippable (and thus all-visible) block.
+ * in vacskip->next_unskippable_block and next_unskippable_allvis. The caller
+ * should not concern itself with anything in vacskip. This is only used by
+ * lazy_scan_skip() to keep track of this state across invocations.
+ *
+ * lazy_scan_skip() returns the next block for vacuum to process and sets its
+ * visibility status in the output parameter, all_visible_according_to_vm.
+ *
+ * vacrel is an in/out parameter here; vacuum options and information about the
+ * relation are read and vacrel->skippedallvis is set to ensure we don't
+ * advance relfrozenxid when we have skipped vacuuming all visible blocks.
+ *
+ * vmbuffer will contain the block from the VM containing visibility
+ * information for the next unskippable heap block. We may end up needed a
+ * different block from the VM (if we decide not to skip a skippable block).
+ * This is okay; visibilitymap_pin() will take care of this while processing
+ * the block.
*
* Note: our opinion of which blocks can be skipped can go stale immediately.
* It's okay if caller "misses" a page whose all-visible or all-frozen marking
@@ -1321,87 +1308,104 @@ lazy_scan_heap(LVRelState *vacrel)
* older XIDs/MXIDs. The vacrel->skippedallvis flag will be set here when the
* choice to skip such a range is actually made, making everything safe.)
*/
-static void
+static BlockNumber
lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
- BlockNumber next_block, Buffer *vmbuffer)
+ BlockNumber next_block, Buffer *vmbuffer,
+ bool *all_visible_according_to_vm)
{
bool skipsallvis = false;
- vacskip->next_unskippable_block = next_block;
- vacskip->next_unskippable_allvis = true;
- while (vacskip->next_unskippable_block < vacrel->rel_pages)
- {
- uint8 mapbits = visibilitymap_get_status(vacrel->rel,
- vacskip->next_unskippable_block,
- vmbuffer);
+ if (next_block >= vacrel->rel_pages)
+ return InvalidBlockNumber;
- if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
+ if (vacskip->next_unskippable_block == InvalidBlockNumber ||
+ next_block > vacskip->next_unskippable_block)
+ {
+ while (++vacskip->next_unskippable_block < vacrel->rel_pages)
{
- Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0);
- vacskip->next_unskippable_allvis = false;
- break;
- }
+ uint8 mapbits = visibilitymap_get_status(vacrel->rel,
+ vacskip->next_unskippable_block,
+ vmbuffer);
- /*
- * Caller must scan the last page to determine whether it has tuples
- * (caller must have the opportunity to set vacrel->nonempty_pages).
- * This rule avoids having lazy_truncate_heap() take access-exclusive
- * lock on rel to attempt a truncation that fails anyway, just because
- * there are tuples on the last page (it is likely that there will be
- * tuples on other nearby pages as well, but those can be skipped).
- *
- * Implement this by always treating the last block as unsafe to skip.
- */
- if (vacskip->next_unskippable_block == vacrel->rel_pages - 1)
- break;
+ vacskip->next_unskippable_allvis = mapbits & VISIBILITYMAP_ALL_VISIBLE;
- /* DISABLE_PAGE_SKIPPING makes all skipping unsafe */
- if (!vacrel->skipwithvm)
- {
- /* Caller shouldn't rely on all_visible_according_to_vm */
- vacskip->next_unskippable_allvis = false;
- break;
- }
+ if (!vacskip->next_unskippable_allvis)
+ {
+ Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0);
+ break;
+ }
- /*
- * Aggressive VACUUM caller can't skip pages just because they are
- * all-visible. They may still skip all-frozen pages, which can't
- * contain XIDs < OldestXmin (XIDs that aren't already frozen by now).
- */
- if ((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0)
- {
- if (vacrel->aggressive)
+ /*
+ * Caller must scan the last page to determine whether it has
+ * tuples (caller must have the opportunity to set
+ * vacrel->nonempty_pages). This rule avoids having
+ * lazy_truncate_heap() take access-exclusive lock on rel to
+ * attempt a truncation that fails anyway, just because there are
+ * tuples on the last page (it is likely that there will be tuples
+ * on other nearby pages as well, but those can be skipped).
+ *
+ * Implement this by always treating the last block as unsafe to
+ * skip.
+ */
+ if (vacskip->next_unskippable_block == vacrel->rel_pages - 1)
break;
+ /* DISABLE_PAGE_SKIPPING makes all skipping unsafe */
+ if (!vacrel->skipwithvm)
+ {
+ /* Caller shouldn't rely on all_visible_according_to_vm */
+ vacskip->next_unskippable_allvis = false;
+ break;
+ }
+
/*
- * All-visible block is safe to skip in non-aggressive case. But
- * remember that the final range contains such a block for later.
+ * Aggressive VACUUM caller can't skip pages just because they are
+ * all-visible. They may still skip all-frozen pages, which can't
+ * contain XIDs < OldestXmin (XIDs that aren't already frozen by
+ * now).
*/
- skipsallvis = true;
+ if ((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0)
+ {
+ if (vacrel->aggressive)
+ break;
+
+ /*
+ * All-visible block is safe to skip in non-aggressive case.
+ * But remember that the final range contains such a block for
+ * later.
+ */
+ skipsallvis = true;
+ }
+
+ vacuum_delay_point();
}
- vacuum_delay_point();
- vacskip->next_unskippable_block++;
+ /*
+ * We only skip a range with at least SKIP_PAGES_THRESHOLD consecutive
+ * pages. Since we're reading sequentially, the OS should be doing
+ * readahead for us, so there's no gain in skipping a page now and
+ * then. Skipping such a range might even discourage sequential
+ * detection.
+ *
+ * This test also enables more frequent relfrozenxid advancement
+ * during non-aggressive VACUUMs. If the range has any all-visible
+ * pages then skipping makes updating relfrozenxid unsafe, which is a
+ * real downside.
+ */
+ if (vacskip->next_unskippable_block - next_block >= SKIP_PAGES_THRESHOLD)
+ {
+ next_block = vacskip->next_unskippable_block;
+ if (skipsallvis)
+ vacrel->skippedallvis = true;
+ }
}
- /*
- * We only skip a range with at least SKIP_PAGES_THRESHOLD consecutive
- * pages. Since we're reading sequentially, the OS should be doing
- * readahead for us, so there's no gain in skipping a page now and then.
- * Skipping such a range might even discourage sequential detection.
- *
- * This test also enables more frequent relfrozenxid advancement during
- * non-aggressive VACUUMs. If the range has any all-visible pages then
- * skipping makes updating relfrozenxid unsafe, which is a real downside.
- */
- if (vacskip->next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD)
- vacskip->skipping_current_range = false;
+ if (next_block == vacskip->next_unskippable_block)
+ *all_visible_according_to_vm = vacskip->next_unskippable_allvis;
else
- {
- vacskip->skipping_current_range = true;
- if (skipsallvis)
- vacrel->skippedallvis = true;
- }
+ *all_visible_according_to_vm = true;
+
+ return next_block;
}
/*
--
2.37.2
From 2fdb5fc93e8db3f885fc270a6742b8bd4c399aab Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 31 Dec 2023 09:47:18 -0500
Subject: [PATCH v1 5/7] VacSkipState saves reference to LVRelState
The streaming read interface can only give pgsr_next callbacks access to
two pieces of private data. As such, move a reference to the LVRelState
into the VacSkipState.
---
src/backend/access/heap/vacuumlazy.c | 36 ++++++++++++++++------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 5070c3fe744..67020c2a807 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -248,11 +248,13 @@ typedef struct VacSkipState
BlockNumber next_unskippable_block;
/* Next unskippable block's visibility status */
bool next_unskippable_allvis;
+ /* reference to whole relation vac state */
+ LVRelState *vacrel;
} VacSkipState;
/* non-export function prototypes */
static void lazy_scan_heap(LVRelState *vacrel);
-static BlockNumber lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
+static BlockNumber lazy_scan_skip(VacSkipState *vacskip,
BlockNumber blkno, Buffer *vmbuffer,
bool *all_visible_according_to_vm);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
@@ -840,7 +842,10 @@ lazy_scan_heap(LVRelState *vacrel)
/* relies on InvalidBlockNumber overflowing to 0 */
BlockNumber blkno = InvalidBlockNumber;
- VacSkipState vacskip = {.next_unskippable_block = InvalidBlockNumber};
+ VacSkipState vacskip = {
+ .next_unskippable_block = InvalidBlockNumber,
+ .vacrel = vacrel
+ };
VacDeadItems *dead_items = vacrel->dead_items;
Buffer vmbuffer = InvalidBuffer;
const int initprog_index[] = {
@@ -862,8 +867,8 @@ lazy_scan_heap(LVRelState *vacrel)
Page page;
LVPagePruneState prunestate;
- blkno = lazy_scan_skip(vacrel, &vacskip, blkno + 1,
- &vmbuffer, &all_visible_according_to_vm);
+ blkno = lazy_scan_skip(&vacskip, blkno + 1,
+ &vmbuffer, &all_visible_according_to_vm);
if (blkno == InvalidBlockNumber)
break;
@@ -1290,9 +1295,10 @@ lazy_scan_heap(LVRelState *vacrel)
* lazy_scan_skip() returns the next block for vacuum to process and sets its
* visibility status in the output parameter, all_visible_according_to_vm.
*
- * vacrel is an in/out parameter here; vacuum options and information about the
- * relation are read and vacrel->skippedallvis is set to ensure we don't
- * advance relfrozenxid when we have skipped vacuuming all visible blocks.
+ * vacskip->vacrel is an in/out parameter here; vacuum options and information
+ * about the relation are read and vacrel->skippedallvis is set to ensure we
+ * don't advance relfrozenxid when we have skipped vacuuming all visible
+ * blocks.
*
* vmbuffer will contain the block from the VM containing visibility
* information for the next unskippable heap block. We may end up needed a
@@ -1309,21 +1315,21 @@ lazy_scan_heap(LVRelState *vacrel)
* choice to skip such a range is actually made, making everything safe.)
*/
static BlockNumber
-lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
+lazy_scan_skip(VacSkipState *vacskip,
BlockNumber next_block, Buffer *vmbuffer,
bool *all_visible_according_to_vm)
{
bool skipsallvis = false;
- if (next_block >= vacrel->rel_pages)
+ if (next_block >= vacskip->vacrel->rel_pages)
return InvalidBlockNumber;
if (vacskip->next_unskippable_block == InvalidBlockNumber ||
next_block > vacskip->next_unskippable_block)
{
- while (++vacskip->next_unskippable_block < vacrel->rel_pages)
+ while (++vacskip->next_unskippable_block < vacskip->vacrel->rel_pages)
{
- uint8 mapbits = visibilitymap_get_status(vacrel->rel,
+ uint8 mapbits = visibilitymap_get_status(vacskip->vacrel->rel,
vacskip->next_unskippable_block,
vmbuffer);
@@ -1347,11 +1353,11 @@ lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
* Implement this by always treating the last block as unsafe to
* skip.
*/
- if (vacskip->next_unskippable_block == vacrel->rel_pages - 1)
+ if (vacskip->next_unskippable_block == vacskip->vacrel->rel_pages - 1)
break;
/* DISABLE_PAGE_SKIPPING makes all skipping unsafe */
- if (!vacrel->skipwithvm)
+ if (!vacskip->vacrel->skipwithvm)
{
/* Caller shouldn't rely on all_visible_according_to_vm */
vacskip->next_unskippable_allvis = false;
@@ -1366,7 +1372,7 @@ lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
*/
if ((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0)
{
- if (vacrel->aggressive)
+ if (vacskip->vacrel->aggressive)
break;
/*
@@ -1396,7 +1402,7 @@ lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
{
next_block = vacskip->next_unskippable_block;
if (skipsallvis)
- vacrel->skippedallvis = true;
+ vacskip->vacrel->skippedallvis = true;
}
}
--
2.37.2
From a9d0592b7a54686968c4872323f5e45b809379c2 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 31 Dec 2023 11:20:07 -0500
Subject: [PATCH v1 6/7] VacSkipState store next unskippable block vmbuffer
This fits nicely with the other state in VacSkipState.
---
src/backend/access/heap/vacuumlazy.c | 47 ++++++++++++++++------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 67020c2a807..49d16fcf039 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -248,6 +248,8 @@ typedef struct VacSkipState
BlockNumber next_unskippable_block;
/* Next unskippable block's visibility status */
bool next_unskippable_allvis;
+ /* Next unskippable block's vmbuffer */
+ Buffer vmbuffer;
/* reference to whole relation vac state */
LVRelState *vacrel;
} VacSkipState;
@@ -255,7 +257,7 @@ typedef struct VacSkipState
/* non-export function prototypes */
static void lazy_scan_heap(LVRelState *vacrel);
static BlockNumber lazy_scan_skip(VacSkipState *vacskip,
- BlockNumber blkno, Buffer *vmbuffer,
+ BlockNumber blkno,
bool *all_visible_according_to_vm);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
@@ -844,10 +846,10 @@ lazy_scan_heap(LVRelState *vacrel)
BlockNumber blkno = InvalidBlockNumber;
VacSkipState vacskip = {
.next_unskippable_block = InvalidBlockNumber,
+ .vmbuffer = InvalidBuffer,
.vacrel = vacrel
};
VacDeadItems *dead_items = vacrel->dead_items;
- Buffer vmbuffer = InvalidBuffer;
const int initprog_index[] = {
PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_TOTAL_HEAP_BLKS,
@@ -868,7 +870,7 @@ lazy_scan_heap(LVRelState *vacrel)
LVPagePruneState prunestate;
blkno = lazy_scan_skip(&vacskip, blkno + 1,
- &vmbuffer, &all_visible_according_to_vm);
+ &all_visible_according_to_vm);
if (blkno == InvalidBlockNumber)
break;
@@ -909,10 +911,10 @@ lazy_scan_heap(LVRelState *vacrel)
* correctness, but we do it anyway to avoid holding the pin
* across a lengthy, unrelated operation.
*/
- if (BufferIsValid(vmbuffer))
+ if (BufferIsValid(vacskip.vmbuffer))
{
- ReleaseBuffer(vmbuffer);
- vmbuffer = InvalidBuffer;
+ ReleaseBuffer(vacskip.vmbuffer);
+ vacskip.vmbuffer = InvalidBuffer;
}
/* Perform a round of index and heap vacuuming */
@@ -937,7 +939,7 @@ lazy_scan_heap(LVRelState *vacrel)
* all-visible. In most cases this will be very cheap, because we'll
* already have the correct page pinned anyway.
*/
- visibilitymap_pin(vacrel->rel, blkno, &vmbuffer);
+ visibilitymap_pin(vacrel->rel, blkno, &vacskip.vmbuffer);
/*
* We need a buffer cleanup lock to prune HOT chains and defragment
@@ -957,7 +959,7 @@ lazy_scan_heap(LVRelState *vacrel)
/* Check for new or empty pages before lazy_scan_noprune call */
if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, true,
- vmbuffer))
+ vacskip.vmbuffer))
{
/* Processed as new/empty page (lock and pin released) */
continue;
@@ -995,7 +997,8 @@ lazy_scan_heap(LVRelState *vacrel)
}
/* Check for new or empty pages before lazy_scan_prune call */
- if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, false, vmbuffer))
+ if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, false,
+ vacskip.vmbuffer))
{
/* Processed as new/empty page (lock and pin released) */
continue;
@@ -1032,7 +1035,7 @@ lazy_scan_heap(LVRelState *vacrel)
{
Size freespace;
- lazy_vacuum_heap_page(vacrel, blkno, buf, 0, vmbuffer);
+ lazy_vacuum_heap_page(vacrel, blkno, buf, 0, vacskip.vmbuffer);
/* Forget the LP_DEAD items that we just vacuumed */
dead_items->num_items = 0;
@@ -1111,7 +1114,7 @@ lazy_scan_heap(LVRelState *vacrel)
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, prunestate.visibility_cutoff_xid,
+ vacskip.vmbuffer, prunestate.visibility_cutoff_xid,
flags);
}
@@ -1122,11 +1125,12 @@ lazy_scan_heap(LVRelState *vacrel)
* with buffer lock before concluding that the VM is corrupt.
*/
else if (all_visible_according_to_vm && !PageIsAllVisible(page) &&
- visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer) != 0)
+ visibilitymap_get_status(vacrel->rel,
+ blkno, &vacskip.vmbuffer) != 0)
{
elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
vacrel->relname, blkno);
- visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
+ visibilitymap_clear(vacrel->rel, blkno, vacskip.vmbuffer,
VISIBILITYMAP_VALID_BITS);
}
@@ -1151,7 +1155,7 @@ lazy_scan_heap(LVRelState *vacrel)
vacrel->relname, blkno);
PageClearAllVisible(page);
MarkBufferDirty(buf);
- visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
+ visibilitymap_clear(vacrel->rel, blkno, vacskip.vmbuffer,
VISIBILITYMAP_VALID_BITS);
}
@@ -1162,7 +1166,7 @@ lazy_scan_heap(LVRelState *vacrel)
*/
else if (all_visible_according_to_vm && prunestate.all_visible &&
prunestate.all_frozen &&
- !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
+ !VM_ALL_FROZEN(vacrel->rel, blkno, &vacskip.vmbuffer))
{
/*
* Avoid relying on all_visible_according_to_vm as a proxy for the
@@ -1184,7 +1188,7 @@ lazy_scan_heap(LVRelState *vacrel)
*/
Assert(!TransactionIdIsValid(prunestate.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, InvalidTransactionId,
+ vacskip.vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
VISIBILITYMAP_ALL_FROZEN);
}
@@ -1226,8 +1230,11 @@ lazy_scan_heap(LVRelState *vacrel)
}
vacrel->blkno = InvalidBlockNumber;
- if (BufferIsValid(vmbuffer))
- ReleaseBuffer(vmbuffer);
+ if (BufferIsValid(vacskip.vmbuffer))
+ {
+ ReleaseBuffer(vacskip.vmbuffer);
+ vacskip.vmbuffer = InvalidBuffer;
+ }
/* report that everything is now scanned */
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
@@ -1316,7 +1323,7 @@ lazy_scan_heap(LVRelState *vacrel)
*/
static BlockNumber
lazy_scan_skip(VacSkipState *vacskip,
- BlockNumber next_block, Buffer *vmbuffer,
+ BlockNumber next_block,
bool *all_visible_according_to_vm)
{
bool skipsallvis = false;
@@ -1331,7 +1338,7 @@ lazy_scan_skip(VacSkipState *vacskip,
{
uint8 mapbits = visibilitymap_get_status(vacskip->vacrel->rel,
vacskip->next_unskippable_block,
- vmbuffer);
+ &vacskip->vmbuffer);
vacskip->next_unskippable_allvis = mapbits & VISIBILITYMAP_ALL_VISIBLE;
--
2.37.2
From b1fe24867172da232456b7e452178bf8adbf0538 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 31 Dec 2023 12:49:56 -0500
Subject: [PATCH v1 7/7] Remove unneeded vacuum_delay_point from lazy_scan_skip
lazy_scan_skip() does relatively little work, so there is no need to
call vacuum_delay_point(). A future commit will call lazy_scan_skip()
from a callback, and we would like to avoid calling vacuum_delay_point()
in that callback.
---
src/backend/access/heap/vacuumlazy.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 49d16fcf039..fc32610397b 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1389,8 +1389,6 @@ lazy_scan_skip(VacSkipState *vacskip,
*/
skipsallvis = true;
}
-
- vacuum_delay_point();
}
/*
--
2.37.2
