agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Melanie Plageman <melanieplageman@gmail.com>
Subject: [PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState
Date: Sat, 30 Dec 2023 16:22:12 -0500
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 add a struct to LVRelState containing variables needed to skip
ranges less than SKIP_PAGES_THRESHOLD.
lazy_scan_prune() and lazy_scan_new_or_empty() can now access the
buffer containing the relevant block of the visibility map through the
LVRelState.skip, so it no longer needs to be a separate function
parameter.
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 | 154 ++++++++++++++++-----------
1 file changed, 90 insertions(+), 64 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 1dc6cc8e4db..0ddb986bc03 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -204,6 +204,22 @@ typedef struct LVRelState
int64 live_tuples; /* # live tuples remaining */
int64 recently_dead_tuples; /* # dead, but not yet removable */
int64 missed_dead_tuples; /* # removable, but not removed */
+
+ /*
+ * Parameters maintained by lazy_scan_skip() to manage skipping ranges of
+ * pages greater than SKIP_PAGES_THRESHOLD.
+ */
+ struct
+ {
+ /* 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;
+ } skip;
} LVRelState;
/* Struct for saving and restoring vacuum error information. */
@@ -214,19 +230,15 @@ typedef struct LVSavedErrInfo
VacErrPhase phase;
} LVSavedErrInfo;
-
/* 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, BlockNumber next_block);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
- bool sharelock, Buffer vmbuffer);
+ bool sharelock);
static void lazy_scan_prune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
- Buffer vmbuffer, bool all_visible_according_to_vm,
+ bool all_visible_according_to_vm,
bool *has_lpdead_items);
static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
@@ -803,12 +815,8 @@ lazy_scan_heap(LVRelState *vacrel)
{
BlockNumber rel_pages = vacrel->rel_pages,
blkno,
- next_unskippable_block,
next_fsm_block_to_vacuum = 0;
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,
@@ -822,10 +830,9 @@ lazy_scan_heap(LVRelState *vacrel)
initprog_val[2] = dead_items->max_items;
pgstat_progress_update_multi_param(3, initprog_index, initprog_val);
+ vacrel->skip.vmbuffer = InvalidBuffer;
/* 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, 0);
for (blkno = 0; blkno < rel_pages; blkno++)
{
Buffer buf;
@@ -834,26 +841,23 @@ lazy_scan_heap(LVRelState *vacrel)
bool has_lpdead_items;
bool got_cleanup_lock = false;
- if (blkno == next_unskippable_block)
+ if (blkno == vacrel->skip.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 = vacrel->skip.next_unskippable_allvis;
+ lazy_scan_skip(vacrel, blkno + 1);
- Assert(next_unskippable_block >= blkno + 1);
+ Assert(vacrel->skip.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 (vacrel->skip.skipping_current_range)
continue;
/* Current range is too small to skip -- just scan the page */
@@ -896,10 +900,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(vacrel->skip.vmbuffer))
{
- ReleaseBuffer(vmbuffer);
- vmbuffer = InvalidBuffer;
+ ReleaseBuffer(vacrel->skip.vmbuffer);
+ vacrel->skip.vmbuffer = InvalidBuffer;
}
/* Perform a round of index and heap vacuuming */
@@ -924,7 +928,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, &vacrel->skip.vmbuffer);
buf = ReadBufferExtended(vacrel->rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
vacrel->bstrategy);
@@ -942,8 +946,7 @@ lazy_scan_heap(LVRelState *vacrel)
LockBuffer(buf, BUFFER_LOCK_SHARE);
/* Check for new or empty pages before lazy_scan_[no]prune call */
- if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, !got_cleanup_lock,
- vmbuffer))
+ if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, !got_cleanup_lock))
{
/* Processed as new/empty page (lock and pin released) */
continue;
@@ -985,7 +988,7 @@ lazy_scan_heap(LVRelState *vacrel)
*/
if (got_cleanup_lock)
lazy_scan_prune(vacrel, buf, blkno, page,
- vmbuffer, all_visible_according_to_vm,
+ all_visible_according_to_vm,
&has_lpdead_items);
/*
@@ -1035,8 +1038,11 @@ lazy_scan_heap(LVRelState *vacrel)
}
vacrel->blkno = InvalidBlockNumber;
- if (BufferIsValid(vmbuffer))
- ReleaseBuffer(vmbuffer);
+ if (BufferIsValid(vacrel->skip.vmbuffer))
+ {
+ ReleaseBuffer(vacrel->skip.vmbuffer);
+ vacrel->skip.vmbuffer = InvalidBuffer;
+ }
/* report that everything is now scanned */
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
@@ -1080,15 +1086,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 skip.
+ * 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.
+ *
+ * skip->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 skip->next_unskippable_block and next_unskippable_allvis.
+ * skip->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
@@ -1098,25 +1123,26 @@ 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, BlockNumber next_block)
{
+ /* Use local variables for better optimized loop code */
BlockNumber rel_pages = vacrel->rel_pages,
next_unskippable_block = next_block;
+
bool skipsallvis = false;
- *next_unskippable_allvis = true;
+ vacrel->skip.next_unskippable_allvis = true;
while (next_unskippable_block < rel_pages)
{
uint8 mapbits = visibilitymap_get_status(vacrel->rel,
next_unskippable_block,
- vmbuffer);
+ &vacrel->skip.vmbuffer);
if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
{
Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0);
- *next_unskippable_allvis = false;
+ vacrel->skip.next_unskippable_allvis = false;
break;
}
@@ -1137,7 +1163,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
if (!vacrel->skipwithvm)
{
/* Caller shouldn't rely on all_visible_according_to_vm */
- *next_unskippable_allvis = false;
+ vacrel->skip.next_unskippable_allvis = false;
break;
}
@@ -1162,6 +1188,8 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
next_unskippable_block++;
}
+ vacrel->skip.next_unskippable_block = 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
@@ -1172,16 +1200,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 (vacrel->skip.next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD)
+ vacrel->skip.skipping_current_range = false;
else
{
- *skipping_current_range = true;
+ vacrel->skip.skipping_current_range = true;
if (skipsallvis)
vacrel->skippedallvis = true;
}
-
- return next_unskippable_block;
}
/*
@@ -1214,7 +1240,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block,
*/
static bool
lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
- Page page, bool sharelock, Buffer vmbuffer)
+ Page page, bool sharelock)
{
Size freespace;
@@ -1300,7 +1326,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
PageSetAllVisible(page);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, InvalidTransactionId,
+ vacrel->skip.vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
END_CRIT_SECTION();
}
@@ -1336,10 +1362,11 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
* any tuple that becomes dead after the call to heap_page_prune() can't need to
* be frozen, because it was visible to another session when vacuum started.
*
- * vmbuffer is the buffer containing the VM block with visibility information
- * for the heap block, blkno. all_visible_according_to_vm is the saved
- * visibility status of the heap block looked up earlier by the caller. We
- * won't rely entirely on this status, as it may be out of date.
+ * vacrel->skipstate.vmbuffer is the buffer containing the VM block with
+ * visibility information for the heap block, blkno.
+ * all_visible_according_to_vm is the saved visibility status of the heap block
+ * looked up earlier by the caller. We won't rely entirely on this status, as
+ * it may be out of date.
*
* *has_lpdead_items is set to true or false depending on whether, upon return
* from this function, any LP_DEAD items are still present on the page.
@@ -1349,7 +1376,6 @@ lazy_scan_prune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
Page page,
- Buffer vmbuffer,
bool all_visible_according_to_vm,
bool *has_lpdead_items)
{
@@ -1783,7 +1809,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vacrel->skip.vmbuffer, visibility_cutoff_xid,
flags);
}
@@ -1794,11 +1820,11 @@ lazy_scan_prune(LVRelState *vacrel,
* 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, &vacrel->skip.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, vacrel->skip.vmbuffer,
VISIBILITYMAP_VALID_BITS);
}
@@ -1822,7 +1848,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->relname, blkno);
PageClearAllVisible(page);
MarkBufferDirty(buf);
- visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
+ visibilitymap_clear(vacrel->rel, blkno, vacrel->skip.vmbuffer,
VISIBILITYMAP_VALID_BITS);
}
@@ -1832,7 +1858,7 @@ lazy_scan_prune(LVRelState *vacrel,
* true, so we must check both all_visible and all_frozen.
*/
else if (all_visible_according_to_vm && all_visible &&
- all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
+ all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vacrel->skip.vmbuffer))
{
/*
* Avoid relying on all_visible_according_to_vm as a proxy for the
@@ -1854,7 +1880,7 @@ lazy_scan_prune(LVRelState *vacrel,
*/
Assert(!TransactionIdIsValid(visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, InvalidTransactionId,
+ vacrel->skip.vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
VISIBILITYMAP_ALL_FROZEN);
}
--
2.40.1
--lb5e4aqx4b6ik5lq
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5a-0003-Confine-vacuum-skip-logic-to-lazy_scan_skip.patch"
view thread (3+ messages) latest in thread
Message-ID: <no-message-id-888186@localhost>
Permalink: ../../no-message-id-888186@localhost/
Also on: postgresql.org/message-id/no-message-id-888186@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: melanieplageman@gmail.com
Subject: Re: [PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState
In-Reply-To: <no-message-id-888186@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox