On Sun, Dec 31, 2023 at 1:28 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:
>
> 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.
I've updated this patch set to remove a commit that didn't make sense
on its own and do various other cleanup.
- Melanie
From 335faad5948b2bec3b83c2db809bb9161d373dcb Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 30 Dec 2023 16:59:27 -0500
Subject: [PATCH v2 4/6] 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.
---
src/backend/access/heap/vacuumlazy.c | 233 ++++++++++++++-------------
1 file changed, 120 insertions(+), 113 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index e3827a5e4d3..42da4ac64f8 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -250,14 +250,13 @@ typedef struct VacSkipState
Buffer vmbuffer;
/* 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);
+static BlockNumber lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
+ BlockNumber blkno,
+ 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);
@@ -838,9 +837,15 @@ static void
lazy_scan_heap(LVRelState *vacrel)
{
BlockNumber rel_pages = vacrel->rel_pages,
- blkno,
next_fsm_block_to_vacuum = 0;
- VacSkipState vacskip = {.vmbuffer = InvalidBuffer};
+ bool all_visible_according_to_vm;
+
+ /* relies on InvalidBlockNumber overflowing to 0 */
+ BlockNumber blkno = InvalidBlockNumber;
+ VacSkipState vacskip = {
+ .next_unskippable_block = InvalidBlockNumber,
+ .vmbuffer = InvalidBuffer
+ };
VacDeadItems *dead_items = vacrel->dead_items;
const int initprog_index[] = {
PROGRESS_VACUUM_PHASE,
@@ -855,37 +860,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);
- 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);
-
- 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,
+ &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++;
@@ -1287,20 +1272,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
*
- * vacskip->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
@@ -1310,14 +1288,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.
+ *
+ * vacskip->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
@@ -1327,87 +1317,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)
+ BlockNumber next_block,
+ 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,
- &vacskip->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,
+ &vacskip->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 eea3c207eeaf7c390096dcb48fc062d81d6d7cc3 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sat, 30 Dec 2023 16:22:12 -0500
Subject: [PATCH v2 3/6] 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 | 145 ++++++++++++++++-----------
src/tools/pgindent/typedefs.list | 1 +
2 files changed, 87 insertions(+), 59 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 3b28ea2cdb5..e3827a5e4d3 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -238,13 +238,26 @@ 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;
+ /* Buffer containing next unskippable block's visibility info */
+ Buffer vmbuffer;
+ /* 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);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool sharelock, Buffer vmbuffer);
@@ -826,12 +839,9 @@ lazy_scan_heap(LVRelState *vacrel)
{
BlockNumber rel_pages = vacrel->rel_pages,
blkno,
- next_unskippable_block,
next_fsm_block_to_vacuum = 0;
+ VacSkipState vacskip = {.vmbuffer = InvalidBuffer};
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 +856,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);
for (blkno = 0; blkno < rel_pages; blkno++)
{
Buffer buf;
@@ -856,26 +864,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);
- 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 */
@@ -918,10 +923,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 */
@@ -946,7 +951,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
@@ -966,7 +971,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;
@@ -1004,7 +1009,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;
@@ -1041,7 +1047,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;
@@ -1120,7 +1126,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);
}
@@ -1131,11 +1137,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);
}
@@ -1160,7 +1167,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);
}
@@ -1171,7 +1178,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
@@ -1193,7 +1200,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);
}
@@ -1235,8 +1242,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);
@@ -1280,15 +1290,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.
+ *
+ * vacskip->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 +1327,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)
{
- 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,
- vmbuffer);
+ vacskip->next_unskippable_block,
+ &vacskip->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 +1358,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 +1387,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
}
vacuum_delay_point();
- next_unskippable_block++;
+ vacskip->next_unskippable_block++;
}
/*
@@ -1371,16 +1400,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 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 v2 1/6] 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 v2 2/6] 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 b6603e35147c4bbe3337280222e6243524b0110e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 31 Dec 2023 09:47:18 -0500
Subject: [PATCH v2 5/6] 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.
This is a separate commit (as opposed to as part of the commit
introducing VacSkipState) because it is required for using the streaming
read interface but not a natural change on its own. VacSkipState is per
block and the LVRelState is referenced for the whole relation vacuum.
---
src/backend/access/heap/vacuumlazy.c | 35 +++++++++++++++-------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 42da4ac64f8..1b64b9988de 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -250,11 +250,13 @@ typedef struct VacSkipState
Buffer vmbuffer;
/* 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,
bool *all_visible_according_to_vm);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
@@ -844,7 +846,8 @@ lazy_scan_heap(LVRelState *vacrel)
BlockNumber blkno = InvalidBlockNumber;
VacSkipState vacskip = {
.next_unskippable_block = InvalidBlockNumber,
- .vmbuffer = InvalidBuffer
+ .vmbuffer = InvalidBuffer,
+ .vacrel = vacrel
};
VacDeadItems *dead_items = vacrel->dead_items;
const int initprog_index[] = {
@@ -866,7 +869,7 @@ lazy_scan_heap(LVRelState *vacrel)
Page page;
LVPagePruneState prunestate;
- blkno = lazy_scan_skip(vacrel, &vacskip, blkno + 1,
+ blkno = lazy_scan_skip(&vacskip, blkno + 1,
&all_visible_according_to_vm);
if (blkno == InvalidBlockNumber)
@@ -1299,9 +1302,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.
*
* vacskip->vmbuffer will contain the block from the VM containing visibility
* information for the next unskippable heap block. We may end up needed a
@@ -1318,21 +1322,20 @@ 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,
- BlockNumber next_block,
- bool *all_visible_according_to_vm)
+lazy_scan_skip(VacSkipState *vacskip,
+ BlockNumber next_block, 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,
&vacskip->vmbuffer);
@@ -1356,11 +1359,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;
@@ -1375,7 +1378,7 @@ lazy_scan_skip(LVRelState *vacrel, VacSkipState *vacskip,
*/
if ((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0)
{
- if (vacrel->aggressive)
+ if (vacskip->vacrel->aggressive)
break;
/*
@@ -1405,7 +1408,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 aa948b99c09f2fdf9a10deac78bcb880e09366ec Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Sun, 31 Dec 2023 12:49:56 -0500
Subject: [PATCH v2 6/6] 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 1b64b9988de..1329da95254 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1388,8 +1388,6 @@ lazy_scan_skip(VacSkipState *vacskip,
*/
skipsallvis = true;
}
-
- vacuum_delay_point();
}
/*
--
2.37.2
