On Thu, Dec 26, 2019 at 09:57:04AM -0600, Justin Pryzby wrote:
> So rebasified against your patch.
Rebased against your patch in master this time.
>From dadb8dff6ea929d78f3695f606de9ade7674b7a1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 27 Nov 2019 20:07:10 -0600
Subject: [PATCH v8 1/5] Rename buf to avoid shadowing buf of another type
---
src/backend/access/heap/vacuumlazy.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a5fe904..de8a89f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -520,7 +520,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
BlockNumber next_unskippable_block;
bool skipping_blocks;
xl_heap_freeze_tuple *frozen;
- StringInfoData buf;
+ StringInfoData sbuf;
const int initprog_index[] = {
PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_TOTAL_HEAP_BLKS,
@@ -1435,33 +1435,33 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
* This is pretty messy, but we split it up so that we can skip emitting
* individual parts of the message when not applicable.
*/
- initStringInfo(&buf);
- appendStringInfo(&buf,
+ initStringInfo(&sbuf);
+ appendStringInfo(&sbuf,
_("%.0f dead row versions cannot be removed yet, oldest xmin: %u\n"),
nkeep, OldestXmin);
- appendStringInfo(&buf, _("There were %.0f unused item identifiers.\n"),
+ appendStringInfo(&sbuf, _("There were %.0f unused item identifiers.\n"),
nunused);
- appendStringInfo(&buf, ngettext("Skipped %u page due to buffer pins, ",
+ appendStringInfo(&sbuf, ngettext("Skipped %u page due to buffer pins, ",
"Skipped %u pages due to buffer pins, ",
vacrelstats->pinskipped_pages),
vacrelstats->pinskipped_pages);
- appendStringInfo(&buf, ngettext("%u frozen page.\n",
+ appendStringInfo(&sbuf, ngettext("%u frozen page.\n",
"%u frozen pages.\n",
vacrelstats->frozenskipped_pages),
vacrelstats->frozenskipped_pages);
- appendStringInfo(&buf, ngettext("%u page is entirely empty.\n",
+ appendStringInfo(&sbuf, ngettext("%u page is entirely empty.\n",
"%u pages are entirely empty.\n",
empty_pages),
empty_pages);
- appendStringInfo(&buf, _("%s."), pg_rusage_show(&ru0));
+ appendStringInfo(&sbuf, _("%s."), pg_rusage_show(&ru0));
ereport(elevel,
(errmsg("\"%s\": found %.0f removable, %.0f nonremovable row versions in %u out of %u pages",
RelationGetRelationName(onerel),
tups_vacuumed, num_tuples,
vacrelstats->scanned_pages, nblocks),
- errdetail_internal("%s", buf.data)));
- pfree(buf.data);
+ errdetail_internal("%s", sbuf.data)));
+ pfree(sbuf.data);
}
/*
--
2.7.4
>From a2b34f259174137ae9f616d9a8e8a57d5b35c4f4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 23 Dec 2019 14:38:01 -0600
Subject: [PATCH v8 2/5] dedup2: skip_blocks
---
src/backend/access/heap/vacuumlazy.c | 187 ++++++++++++++++-------------------
1 file changed, 84 insertions(+), 103 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index de8a89f..b94e052 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -480,6 +480,88 @@ vacuum_log_cleanup_info(Relation rel, LVRelStats *vacrelstats)
}
/*
+ * Return whether skipping blocks or not.
+ * Except when aggressive is set, we want to skip pages that are
+ * all-visible according to the visibility map, but only when we can skip
+ * 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; that's likely to disable
+ * readahead and so be counterproductive. Also, skipping even a single
+ * page means that we can't update relfrozenxid, so we only want to do it
+ * if we can skip a goodly number of pages.
+ *
+ * When aggressive is set, we can't skip pages just because they are
+ * all-visible, but we can still skip pages that are all-frozen, since
+ * such pages do not need freezing and do not affect the value that we can
+ * safely set for relfrozenxid or relminmxid.
+ *
+ * Before entering the main loop, establish the invariant that
+ * next_unskippable_block is the next block number >= blkno that we can't
+ * skip based on the visibility map, either all-visible for a regular scan
+ * or all-frozen for an aggressive scan. We set it to nblocks if there's
+ * no such block. We also set up the skipping_blocks flag correctly at
+ * this stage.
+ *
+ * Note: The value returned by visibilitymap_get_status could be slightly
+ * out-of-date, since we make this test before reading the corresponding
+ * heap page or locking the buffer. This is OK. If we mistakenly think
+ * that the page is all-visible or all-frozen when in fact the flag's just
+ * been cleared, we might fail to vacuum the page. It's easy to see that
+ * skipping a page when aggressive is not set is not a very big deal; we
+ * might leave some dead tuples lying around, but the next vacuum will
+ * find them. But even when aggressive *is* set, it's still OK if we miss
+ * a page whose all-frozen marking has just been cleared. Any new XIDs
+ * just added to that page are necessarily newer than the GlobalXmin we
+ * computed, so they'll have no effect on the value to which we can safely
+ * set relfrozenxid. A similar argument applies for MXIDs and relminmxid.
+ *
+ * We will scan the table's last page, at least to the extent of
+ * determining whether it has tuples or not, even if it should be skipped
+ * according to the above rules; except when we've already determined that
+ * it's not worth trying to truncate the table. This avoids having
+ * lazy_truncate_heap() take access-exclusive lock on the table to attempt
+ * a truncation that just fails immediately because there are tuples in
+ * the last page. This is worth avoiding mainly because such a lock must
+ * be replayed on any hot standby, where it can be disruptive.
+ */
+static bool
+skip_blocks(Relation onerel, VacuumParams *params, BlockNumber *next_unskippable_block, BlockNumber nblocks, Buffer *vmbuffer, bool aggressive)
+{
+ if ((params->options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
+ {
+ while (*next_unskippable_block < nblocks)
+ {
+ uint8 vmstatus;
+
+ vmstatus = visibilitymap_get_status(onerel, *next_unskippable_block,
+ vmbuffer);
+ if (aggressive)
+ {
+ if ((vmstatus & VISIBILITYMAP_ALL_FROZEN) == 0)
+ break;
+ }
+ else
+ {
+ if ((vmstatus & VISIBILITYMAP_ALL_VISIBLE) == 0)
+ break;
+ }
+ vacuum_delay_point();
+ ++*next_unskippable_block;
+ }
+ }
+
+
+ /*
+ * We know we can't skip the current block. But set up
+ * skipping_blocks to do the right thing at the following blocks.
+ */
+ if (*next_unskippable_block >= SKIP_PAGES_THRESHOLD)
+ return true;
+ else
+ return false;
+}
+
+/*
* lazy_scan_heap() -- scan an open heap relation
*
* This routine prunes each page in the heap, which will among other
@@ -565,78 +647,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
initprog_val[2] = vacrelstats->max_dead_tuples;
pgstat_progress_update_multi_param(3, initprog_index, initprog_val);
- /*
- * Except when aggressive is set, we want to skip pages that are
- * all-visible according to the visibility map, but only when we can skip
- * 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; that's likely to disable
- * readahead and so be counterproductive. Also, skipping even a single
- * page means that we can't update relfrozenxid, so we only want to do it
- * if we can skip a goodly number of pages.
- *
- * When aggressive is set, we can't skip pages just because they are
- * all-visible, but we can still skip pages that are all-frozen, since
- * such pages do not need freezing and do not affect the value that we can
- * safely set for relfrozenxid or relminmxid.
- *
- * Before entering the main loop, establish the invariant that
- * next_unskippable_block is the next block number >= blkno that we can't
- * skip based on the visibility map, either all-visible for a regular scan
- * or all-frozen for an aggressive scan. We set it to nblocks if there's
- * no such block. We also set up the skipping_blocks flag correctly at
- * this stage.
- *
- * Note: The value returned by visibilitymap_get_status could be slightly
- * out-of-date, since we make this test before reading the corresponding
- * heap page or locking the buffer. This is OK. If we mistakenly think
- * that the page is all-visible or all-frozen when in fact the flag's just
- * been cleared, we might fail to vacuum the page. It's easy to see that
- * skipping a page when aggressive is not set is not a very big deal; we
- * might leave some dead tuples lying around, but the next vacuum will
- * find them. But even when aggressive *is* set, it's still OK if we miss
- * a page whose all-frozen marking has just been cleared. Any new XIDs
- * just added to that page are necessarily newer than the GlobalXmin we
- * computed, so they'll have no effect on the value to which we can safely
- * set relfrozenxid. A similar argument applies for MXIDs and relminmxid.
- *
- * We will scan the table's last page, at least to the extent of
- * determining whether it has tuples or not, even if it should be skipped
- * according to the above rules; except when we've already determined that
- * it's not worth trying to truncate the table. This avoids having
- * lazy_truncate_heap() take access-exclusive lock on the table to attempt
- * a truncation that just fails immediately because there are tuples in
- * the last page. This is worth avoiding mainly because such a lock must
- * be replayed on any hot standby, where it can be disruptive.
- */
next_unskippable_block = 0;
- if ((params->options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
- {
- while (next_unskippable_block < nblocks)
- {
- uint8 vmstatus;
-
- vmstatus = visibilitymap_get_status(onerel, next_unskippable_block,
- &vmbuffer);
- if (aggressive)
- {
- if ((vmstatus & VISIBILITYMAP_ALL_FROZEN) == 0)
- break;
- }
- else
- {
- if ((vmstatus & VISIBILITYMAP_ALL_VISIBLE) == 0)
- break;
- }
- vacuum_delay_point();
- next_unskippable_block++;
- }
- }
-
- if (next_unskippable_block >= SKIP_PAGES_THRESHOLD)
- skipping_blocks = true;
- else
- skipping_blocks = false;
+ skipping_blocks = skip_blocks(onerel, params, &next_unskippable_block, nblocks, &vmbuffer, aggressive);
for (blkno = 0; blkno < nblocks; blkno++)
{
@@ -665,38 +677,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
{
/* Time to advance next_unskippable_block */
next_unskippable_block++;
- if ((params->options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
- {
- while (next_unskippable_block < nblocks)
- {
- uint8 vmskipflags;
-
- vmskipflags = visibilitymap_get_status(onerel,
- next_unskippable_block,
- &vmbuffer);
- if (aggressive)
- {
- if ((vmskipflags & VISIBILITYMAP_ALL_FROZEN) == 0)
- break;
- }
- else
- {
- if ((vmskipflags & VISIBILITYMAP_ALL_VISIBLE) == 0)
- break;
- }
- vacuum_delay_point();
- next_unskippable_block++;
- }
- }
-
- /*
- * We know we can't skip the current block. But set up
- * skipping_blocks to do the right thing at the following blocks.
- */
- if (next_unskippable_block - blkno > SKIP_PAGES_THRESHOLD)
- skipping_blocks = true;
- else
- skipping_blocks = false;
+ skipping_blocks = skip_blocks(onerel, params, &next_unskippable_block, nblocks, &vmbuffer, aggressive);
/*
* Normally, the fact that we can't skip this block must mean that
--
2.7.4
>From 270634aa0e903289f1ccafe6d72533e93013d078 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 29 Dec 2019 13:38:49 -0600
Subject: [PATCH v8 3/5] vacuumlazy: typos in comments
---
src/backend/access/heap/vacuumlazy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index b94e052..e35fe6a 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1256,7 +1256,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
/*
* It should never be the case that the visibility map page is set
* while the page-level bit is clear, but the reverse is allowed
- * (if checksums are not enabled). Regardless, set the both bits
+ * (if checksums are not enabled). Regardless, set both bits
* so that we get back in sync.
*
* NB: If the heap page is all-visible but the VM bit is not set,
@@ -1312,7 +1312,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
}
/*
- * If the all-visible page is turned out to be all-frozen but not
+ * If the all-visible page turned out to be all-frozen but not
* marked, we should so mark it. Note that all_frozen is only valid
* if all_visible is true, so we must check both.
*/
--
2.7.4
>From e43ef5c8427bf2c42658204e57e68767d0c6a2ad Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 12 Dec 2019 20:54:37 -0600
Subject: [PATCH v8 4/5] vacuum errcontext to show block being processed
As requested here.
https://www.postgresql.org/message-id/20190807235154.erbmr4o4bo6vgnjv%40alap3.anarazel.de
---
src/backend/access/heap/vacuumlazy.c | 37 ++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index e35fe6a..52602e2 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -138,6 +138,12 @@ typedef struct LVRelStats
bool lock_waiter_detected;
} LVRelStats;
+typedef struct
+{
+ char *relname;
+ char *relnamespace;
+ BlockNumber blkno;
+} vacuum_error_callback_arg;
/* A few variables that don't seem worth passing around as parameters */
static int elevel = -1;
@@ -178,6 +184,7 @@ static bool lazy_tid_reaped(ItemPointer itemptr, void *state);
static int vac_cmp_itemptr(const void *left, const void *right);
static bool heap_page_is_all_visible(Relation rel, Buffer buf,
TransactionId *visibility_cutoff_xid, bool *all_frozen);
+static void vacuum_error_callback(void *arg);
/*
@@ -609,6 +616,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
PROGRESS_VACUUM_MAX_DEAD_TUPLES
};
int64 initprog_val[3];
+ ErrorContextCallback errcallback;
+ vacuum_error_callback_arg errcbarg;
pg_rusage_init(&ru0);
@@ -650,6 +659,15 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
next_unskippable_block = 0;
skipping_blocks = skip_blocks(onerel, params, &next_unskippable_block, nblocks, &vmbuffer, aggressive);
+ /* Setup error traceback support for ereport() */
+ errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+ errcbarg.relname = relname;
+ errcbarg.blkno = InvalidBlockNumber; /* Not known yet */
+ errcallback.callback = vacuum_error_callback;
+ errcallback.arg = (void *) &errcbarg;
+ errcallback.previous = error_context_stack;
+ error_context_stack = &errcallback;
+
for (blkno = 0; blkno < nblocks; blkno++)
{
Buffer buf;
@@ -671,6 +689,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
#define FORCE_CHECK_PAGE() \
(blkno == nblocks - 1 && should_attempt_truncation(params, vacrelstats))
+ errcbarg.blkno = blkno;
+
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
if (blkno == next_unskippable_block)
@@ -737,8 +757,11 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
}
/* Work on all the indexes, then the heap */
+ /* Don't use the errcontext handler outside this function */
+ error_context_stack = errcallback.previous;
lazy_vacuum_all_indexes(onerel, vacrelstats, Irel,
nindexes, indstats);
+ error_context_stack = &errcallback;
/* Remove tuples from heap */
lazy_vacuum_heap(onerel, vacrelstats);
@@ -1346,6 +1369,9 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
RecordPageWithFreeSpace(onerel, blkno, freespace);
}
+ /* Pop the error context stack */
+ error_context_stack = errcallback.previous;
+
/* report that everything is scanned and vacuumed */
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
@@ -2323,3 +2349,14 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
return all_visible;
}
+
+/*
+ * Error context callback for errors occurring during vacuum.
+ */
+static void
+vacuum_error_callback(void *arg)
+{
+ vacuum_error_callback_arg *cbarg = arg;
+ errcontext("while scanning block %u of relation \"%s.%s\"",
+ cbarg->blkno, cbarg->relnamespace, cbarg->relname);
+}
--
2.7.4
>From ce2e6f062f5fc800539a662a03c1888eced51d43 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 12 Dec 2019 20:34:03 -0600
Subject: [PATCH v8 5/5] add errcontext callback in lazy_vacuum_heap, too
---
src/backend/access/heap/vacuumlazy.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 52602e2..532698d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1410,6 +1410,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
/* Remove tuples from heap */
lazy_vacuum_heap(onerel, vacrelstats);
+ error_context_stack = errcallback.previous;
}
/*
@@ -1521,10 +1522,22 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
PGRUsage ru0;
Buffer vmbuffer = InvalidBuffer;
+ ErrorContextCallback errcallback;
+ vacuum_error_callback_arg errcbarg;
+
/* Report that we are now vacuuming the heap */
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_PHASE_VACUUM_HEAP);
+ /* Setup error traceback support for ereport() */
+ errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+ errcbarg.relname = RelationGetRelationName(onerel);
+ errcbarg.blkno = InvalidBlockNumber; /* Not known yet */
+ errcallback.callback = vacuum_error_callback;
+ errcallback.arg = (void *) &errcbarg;
+ errcallback.previous = error_context_stack;
+ error_context_stack = &errcallback;
+
pg_rusage_init(&ru0);
npages = 0;
@@ -1539,6 +1552,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
vacuum_delay_point();
tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples[tupindex]);
+ errcbarg.blkno = tblk;
buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL,
vac_strategy);
if (!ConditionalLockBufferForCleanup(buf))
--
2.7.4
