On Tue, Jun 23, 2020 at 12:14:40AM -0400, Tom Lane wrote:
> Andres Freund <and...@anarazel.de> writes:
> > I am only suggesting that where you save the old location, as currently
> > done with LVRelStats olderrinfo, you instead use a more specific
> > type. Not that you should pass that anywhere (except for
> > update_vacuum_error_info).
>
> As things currently stand, I don't think we need another struct
> type at all. ISTM we should hard-wire the handling of indname
> as I suggested above. Then there are only two fields to be dealt
> with, and we could just as well save them in simple local variables.
>
> If there's a clear future path to needing to save/restore more
> fields, then maybe another struct type would be useful ... but
> right now the struct type declaration itself would take more
> lines of code than it would save.
Updated patches for consideration. I left the "struct" patch there to show
what it'd look like.
--
Justin
>From bbfdff2483730d45c663b75de4700e50f09ab4d3 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 22 Jun 2020 18:22:58 -0500
Subject: [PATCH v2 1/5] Rename from "errcbarg"..
..the name of which Alvaro didn't like. I renamed this, but it was
accidentally re-introduced while trading patches with Amit.
We should *also* rename VACUUM_ERRCB* to VACUUM_ERRINFO, but I didn't do that here.
---
src/backend/access/heap/vacuumlazy.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 3bef0e124b..a6a5d906c0 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2459,7 +2459,7 @@ lazy_cleanup_index(Relation indrel,
IndexVacuumInfo ivinfo;
const char *msg;
PGRUsage ru0;
- LVRelStats olderrcbarg;
+ LVRelStats olderrinfo;
pg_rusage_init(&ru0);
@@ -2473,7 +2473,7 @@ lazy_cleanup_index(Relation indrel,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
- olderrcbarg = *vacrelstats;
+ olderrinfo = *vacrelstats;
update_vacuum_error_info(vacrelstats,
VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
InvalidBlockNumber,
@@ -2483,9 +2483,9 @@ lazy_cleanup_index(Relation indrel,
/* Revert back to the old phase information for error traceback */
update_vacuum_error_info(vacrelstats,
- olderrcbarg.phase,
- olderrcbarg.blkno,
- olderrcbarg.indname);
+ olderrinfo.phase,
+ olderrinfo.blkno,
+ olderrinfo.indname);
if (!(*stats))
return;
--
2.17.0
>From afd4916d190d7da48b535432f8edea65615d7fe8 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 22 Jun 2020 18:51:35 -0500
Subject: [PATCH v2 2/5] Specially save the index name in
lazy_{vacuum,cleanup}_index..
..at expense of symmetry, move handling of index outside of
update_vacuum_error_info, since it's more clear this way that use of indname is
limited and safe.
---
src/backend/access/heap/vacuumlazy.c | 49 ++++++++++++----------------
1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a6a5d906c0..43a3c093fd 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -389,7 +389,7 @@ static LVSharedIndStats *get_indstats(LVShared *lvshared, int n);
static bool skip_parallel_vacuum_index(Relation indrel, LVShared *lvshared);
static void vacuum_error_callback(void *arg);
static void update_vacuum_error_info(LVRelStats *errinfo, int phase,
- BlockNumber blkno, char *indname);
+ BlockNumber blkno);
/*
@@ -477,7 +477,6 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
vacrelstats->relname = pstrdup(RelationGetRelationName(onerel));
- vacrelstats->indname = NULL;
vacrelstats->phase = VACUUM_ERRCB_PHASE_UNKNOWN;
vacrelstats->old_rel_pages = onerel->rd_rel->relpages;
vacrelstats->old_live_tuples = onerel->rd_rel->reltuples;
@@ -539,7 +538,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
* revert to the previous phase.
*/
update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_TRUNCATE,
- vacrelstats->nonempty_pages, NULL);
+ vacrelstats->nonempty_pages);
lazy_truncate_heap(onerel, vacrelstats);
}
@@ -949,7 +948,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_SCAN_HEAP,
- blkno, NULL);
+ blkno);
if (blkno == next_unskippable_block)
{
@@ -1829,7 +1828,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
/* Update error traceback information */
olderrinfo = *vacrelstats;
update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
- InvalidBlockNumber, NULL);
+ InvalidBlockNumber);
pg_rusage_init(&ru0);
npages = 0;
@@ -1881,8 +1880,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
/* Revert to the previous phase information for error traceback */
update_vacuum_error_info(vacrelstats,
olderrinfo.phase,
- olderrinfo.blkno,
- olderrinfo.indname);
+ olderrinfo.blkno);
}
/*
@@ -1912,7 +1910,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
/* Update error traceback information */
olderrinfo = *vacrelstats;
update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
- blkno, NULL);
+ blkno);
START_CRIT_SECTION();
@@ -1993,8 +1991,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
/* Revert to the previous phase information for error traceback */
update_vacuum_error_info(vacrelstats,
olderrinfo.phase,
- olderrinfo.blkno,
- olderrinfo.indname);
+ olderrinfo.blkno);
return tupindex;
}
@@ -2418,10 +2415,11 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
/* Update error traceback information */
olderrinfo = *vacrelstats;
+ /* The index name is also saved during this phase */
+ vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
update_vacuum_error_info(vacrelstats,
VACUUM_ERRCB_PHASE_VACUUM_INDEX,
- InvalidBlockNumber,
- RelationGetRelationName(indrel));
+ InvalidBlockNumber);
/* Do bulk deletion */
*stats = index_bulk_delete(&ivinfo, *stats,
@@ -2441,8 +2439,8 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
/* Revert to the previous phase information for error traceback */
update_vacuum_error_info(vacrelstats,
olderrinfo.phase,
- olderrinfo.blkno,
- olderrinfo.indname);
+ olderrinfo.blkno);
+ pfree(vacrelstats->indname);
}
/*
@@ -2474,18 +2472,20 @@ lazy_cleanup_index(Relation indrel,
/* Update error traceback information */
olderrinfo = *vacrelstats;
+ /* The index name is also saved during this phase */
+ vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
update_vacuum_error_info(vacrelstats,
VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
- InvalidBlockNumber,
- RelationGetRelationName(indrel));
+ InvalidBlockNumber);
*stats = index_vacuum_cleanup(&ivinfo, *stats);
/* Revert back to the old phase information for error traceback */
update_vacuum_error_info(vacrelstats,
olderrinfo.phase,
- olderrinfo.blkno,
- olderrinfo.indname);
+ olderrinfo.blkno);
+ pfree(vacrelstats->indname);
+
if (!(*stats))
return;
@@ -3523,7 +3523,6 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
*/
vacrelstats.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
vacrelstats.relname = pstrdup(RelationGetRelationName(onerel));
- vacrelstats.indname = NULL;
vacrelstats.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
/* Setup error traceback support for ereport() */
@@ -3598,18 +3597,10 @@ vacuum_error_callback(void *arg)
}
}
-/* Update vacuum error callback for the current phase, block, and index. */
+/* Update vacuum error callback for the current phase and block. */
static void
-update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno,
- char *indname)
+update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno)
{
errinfo->blkno = blkno;
errinfo->phase = phase;
-
- /* Free index name from any previous phase */
- if (errinfo->indname)
- pfree(errinfo->indname);
-
- /* For index phases, save the name of the current index for the callback */
- errinfo->indname = indname ? pstrdup(indname) : NULL;
}
--
2.17.0
>From f57302143cea857b29b4f19f30bad0e28209c50d Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 23 Jun 2020 08:35:29 -0500
Subject: [PATCH v2 3/5] Save phase/blkno in local variables, for consistency
with indname
---
src/backend/access/heap/vacuumlazy.c | 32 +++++++++++-----------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 43a3c093fd..2393269565 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1819,14 +1819,14 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
int npages;
PGRUsage ru0;
Buffer vmbuffer = InvalidBuffer;
- LVRelStats olderrinfo;
+ int oldphase = vacrelstats->phase,
+ oldblkno = vacrelstats->blkno;
/* Report that we are now vacuuming the heap */
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_PHASE_VACUUM_HEAP);
/* Update error traceback information */
- olderrinfo = *vacrelstats;
update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
InvalidBlockNumber);
@@ -1878,9 +1878,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
errdetail_internal("%s", pg_rusage_show(&ru0))));
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(vacrelstats,
- olderrinfo.phase,
- olderrinfo.blkno);
+ update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
}
/*
@@ -1903,12 +1901,12 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
int uncnt = 0;
TransactionId visibility_cutoff_xid;
bool all_frozen;
- LVRelStats olderrinfo;
+ int oldphase = vacrelstats->phase,
+ oldblkno = vacrelstats->blkno;
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno);
/* Update error traceback information */
- olderrinfo = *vacrelstats;
update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
blkno);
@@ -1989,9 +1987,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
}
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(vacrelstats,
- olderrinfo.phase,
- olderrinfo.blkno);
+ update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
return tupindex;
}
@@ -2401,7 +2397,8 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
IndexVacuumInfo ivinfo;
const char *msg;
PGRUsage ru0;
- LVRelStats olderrinfo;
+ int oldphase = vacrelstats->phase,
+ oldblkno = vacrelstats->blkno;
pg_rusage_init(&ru0);
@@ -2414,7 +2411,6 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
- olderrinfo = *vacrelstats;
/* The index name is also saved during this phase */
vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
update_vacuum_error_info(vacrelstats,
@@ -2437,9 +2433,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
errdetail_internal("%s", pg_rusage_show(&ru0))));
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(vacrelstats,
- olderrinfo.phase,
- olderrinfo.blkno);
+ update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
pfree(vacrelstats->indname);
}
@@ -2457,7 +2451,8 @@ lazy_cleanup_index(Relation indrel,
IndexVacuumInfo ivinfo;
const char *msg;
PGRUsage ru0;
- LVRelStats olderrinfo;
+ int oldphase = vacrelstats->phase,
+ oldblkno = vacrelstats->blkno;
pg_rusage_init(&ru0);
@@ -2471,7 +2466,6 @@ lazy_cleanup_index(Relation indrel,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
- olderrinfo = *vacrelstats;
/* The index name is also saved during this phase */
vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
update_vacuum_error_info(vacrelstats,
@@ -2481,9 +2475,7 @@ lazy_cleanup_index(Relation indrel,
*stats = index_vacuum_cleanup(&ivinfo, *stats);
/* Revert back to the old phase information for error traceback */
- update_vacuum_error_info(vacrelstats,
- olderrinfo.phase,
- olderrinfo.blkno);
+ update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
pfree(vacrelstats->indname);
if (!(*stats))
--
2.17.0
>From 78d378fd22f74828981692e5e253214bccef4311 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 22 Jun 2020 16:36:14 -0500
Subject: [PATCH v2 4/5] Move error information into a separate struct..
..to avoid copying large LVRelStats struct.
Andres things this is a problem but Tom thinks it's a micro-optimization.
See: b61d161c146328ae6ba9ed937862d66e5c8b035a
This also moves relname and relnamespace (see ef75140fe).
Discussion: https://www.postgresql.org/message-id/20200622200939.jkuniq3vtiumeszj%40alap3.anarazel.de
---
src/backend/access/heap/vacuumlazy.c | 128 +++++++++++++++------------
1 file changed, 72 insertions(+), 56 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 2393269565..9eb4bc66ae 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -288,10 +288,17 @@ typedef struct LVParallelState
int nindexes_parallel_condcleanup;
} LVParallelState;
-typedef struct LVRelStats
+typedef struct
{
char *relnamespace;
char *relname;
+ char *indname;
+ BlockNumber blkno; /* used only for heap operations */
+ VacErrPhase phase;
+} LVSavedPosition;
+
+typedef struct LVRelStats
+{
/* useindex = true means two-pass strategy; false means one-pass */
bool useindex;
/* Overall statistics about rel */
@@ -313,10 +320,7 @@ typedef struct LVRelStats
TransactionId latestRemovedXid;
bool lock_waiter_detected;
- /* Used for error callback */
- char *indname;
- BlockNumber blkno; /* used only for heap operations */
- VacErrPhase phase;
+ LVSavedPosition errinfo; /* Used for error callback */
} LVRelStats;
/* A few variables that don't seem worth passing around as parameters */
@@ -388,7 +392,7 @@ static void end_parallel_vacuum(Relation *Irel, IndexBulkDeleteResult **stats,
static LVSharedIndStats *get_indstats(LVShared *lvshared, int n);
static bool skip_parallel_vacuum_index(Relation indrel, LVShared *lvshared);
static void vacuum_error_callback(void *arg);
-static void update_vacuum_error_info(LVRelStats *errinfo, int phase,
+static void update_vacuum_error_info(LVSavedPosition *errinfo, int phase,
BlockNumber blkno);
@@ -475,9 +479,9 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
- vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
- vacrelstats->relname = pstrdup(RelationGetRelationName(onerel));
- vacrelstats->phase = VACUUM_ERRCB_PHASE_UNKNOWN;
+ vacrelstats->errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+ vacrelstats->errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+ vacrelstats->errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN;
vacrelstats->old_rel_pages = onerel->rd_rel->relpages;
vacrelstats->old_live_tuples = onerel->rd_rel->reltuples;
vacrelstats->num_index_scans = 0;
@@ -501,7 +505,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
* information is restored at the end of those phases.
*/
errcallback.callback = vacuum_error_callback;
- errcallback.arg = vacrelstats;
+ errcallback.arg = &vacrelstats->errinfo;
errcallback.previous = error_context_stack;
error_context_stack = &errcallback;
@@ -537,7 +541,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
* which we add context information to errors, so we don't need to
* revert to the previous phase.
*/
- update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_TRUNCATE,
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ VACUUM_ERRCB_PHASE_TRUNCATE,
vacrelstats->nonempty_pages);
lazy_truncate_heap(onerel, vacrelstats);
}
@@ -649,8 +654,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
}
appendStringInfo(&buf, msgfmt,
get_database_name(MyDatabaseId),
- vacrelstats->relnamespace,
- vacrelstats->relname,
+ vacrelstats->errinfo.relnamespace,
+ vacrelstats->errinfo.relname,
vacrelstats->num_index_scans);
appendStringInfo(&buf, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen\n"),
vacrelstats->pages_removed,
@@ -785,13 +790,13 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
if (aggressive)
ereport(elevel,
(errmsg("aggressively vacuuming \"%s.%s\"",
- vacrelstats->relnamespace,
- vacrelstats->relname)));
+ vacrelstats->errinfo.relnamespace,
+ vacrelstats->errinfo.relname)));
else
ereport(elevel,
(errmsg("vacuuming \"%s.%s\"",
- vacrelstats->relnamespace,
- vacrelstats->relname)));
+ vacrelstats->errinfo.relnamespace,
+ vacrelstats->errinfo.relname)));
empty_pages = vacuumed_pages = 0;
next_fsm_block_to_vacuum = (BlockNumber) 0;
@@ -827,7 +832,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
if (params->nworkers > 0)
ereport(WARNING,
(errmsg("disabling parallel option of vacuum on \"%s\" --- cannot vacuum temporary tables in parallel",
- vacrelstats->relname)));
+ vacrelstats->errinfo.relname)));
}
else
lps = begin_parallel_vacuum(RelationGetRelid(onerel), Irel,
@@ -947,7 +952,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
- update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_SCAN_HEAP,
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ VACUUM_ERRCB_PHASE_SCAN_HEAP,
blkno);
if (blkno == next_unskippable_block)
@@ -1581,7 +1587,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
&& VM_ALL_VISIBLE(onerel, blkno, &vmbuffer))
{
elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
- vacrelstats->relname, blkno);
+ vacrelstats->errinfo.relname, blkno);
visibilitymap_clear(onerel, blkno, vmbuffer,
VISIBILITYMAP_VALID_BITS);
}
@@ -1602,7 +1608,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
else if (PageIsAllVisible(page) && has_dead_tuples)
{
elog(WARNING, "page containing dead tuples is marked as all-visible in relation \"%s\" page %u",
- vacrelstats->relname, blkno);
+ vacrelstats->errinfo.relname, blkno);
PageClearAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_clear(onerel, blkno, vmbuffer,
@@ -1712,7 +1718,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
if (vacuumed_pages)
ereport(elevel,
(errmsg("\"%s\": removed %.0f row versions in %u pages",
- vacrelstats->relname,
+ vacrelstats->errinfo.relname,
tups_vacuumed, vacuumed_pages)));
/*
@@ -1741,7 +1747,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
ereport(elevel,
(errmsg("\"%s\": found %.0f removable, %.0f nonremovable row versions in %u out of %u pages",
- vacrelstats->relname,
+ vacrelstats->errinfo.relname,
tups_vacuumed, num_tuples,
vacrelstats->scanned_pages, nblocks),
errdetail_internal("%s", buf.data)));
@@ -1819,15 +1825,16 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
int npages;
PGRUsage ru0;
Buffer vmbuffer = InvalidBuffer;
- int oldphase = vacrelstats->phase,
- oldblkno = vacrelstats->blkno;
+ LVSavedPosition olderrinfo;
/* Report that we are now vacuuming the heap */
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_PHASE_VACUUM_HEAP);
/* Update error traceback information */
- update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
+ olderrinfo = vacrelstats->errinfo;
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ VACUUM_ERRCB_PHASE_VACUUM_HEAP,
InvalidBlockNumber);
pg_rusage_init(&ru0);
@@ -1844,7 +1851,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
vacuum_delay_point();
tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
- vacrelstats->blkno = tblk;
+ vacrelstats->errinfo.blkno = tblk;
buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL,
vac_strategy);
if (!ConditionalLockBufferForCleanup(buf))
@@ -1873,12 +1880,14 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
ereport(elevel,
(errmsg("\"%s\": removed %d row versions in %d pages",
- vacrelstats->relname,
+ vacrelstats->errinfo.relname,
tupindex, npages),
errdetail_internal("%s", pg_rusage_show(&ru0))));
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ olderrinfo.phase,
+ olderrinfo.blkno);
}
/*
@@ -1901,13 +1910,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
int uncnt = 0;
TransactionId visibility_cutoff_xid;
bool all_frozen;
- int oldphase = vacrelstats->phase,
- oldblkno = vacrelstats->blkno;
+ LVSavedPosition olderrinfo;
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno);
/* Update error traceback information */
- update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
+ olderrinfo = vacrelstats->errinfo;
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ VACUUM_ERRCB_PHASE_VACUUM_HEAP,
blkno);
START_CRIT_SECTION();
@@ -1987,7 +1997,9 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
}
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ olderrinfo.phase,
+ olderrinfo.blkno);
return tupindex;
}
@@ -2397,8 +2409,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
IndexVacuumInfo ivinfo;
const char *msg;
PGRUsage ru0;
- int oldphase = vacrelstats->phase,
- oldblkno = vacrelstats->blkno;
+ LVSavedPosition olderrinfo;
pg_rusage_init(&ru0);
@@ -2411,9 +2422,10 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
+ olderrinfo = vacrelstats->errinfo;
/* The index name is also saved during this phase */
- vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
- update_vacuum_error_info(vacrelstats,
+ vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel));
+ update_vacuum_error_info(&vacrelstats->errinfo,
VACUUM_ERRCB_PHASE_VACUUM_INDEX,
InvalidBlockNumber);
@@ -2428,13 +2440,15 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ereport(elevel,
(errmsg(msg,
- vacrelstats->indname,
+ vacrelstats->errinfo.indname,
dead_tuples->num_tuples),
errdetail_internal("%s", pg_rusage_show(&ru0))));
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
- pfree(vacrelstats->indname);
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ olderrinfo.phase,
+ olderrinfo.blkno);
+ pfree(vacrelstats->errinfo.indname);
}
/*
@@ -2451,8 +2465,7 @@ lazy_cleanup_index(Relation indrel,
IndexVacuumInfo ivinfo;
const char *msg;
PGRUsage ru0;
- int oldphase = vacrelstats->phase,
- oldblkno = vacrelstats->blkno;
+ LVSavedPosition olderrinfo;
pg_rusage_init(&ru0);
@@ -2466,17 +2479,20 @@ lazy_cleanup_index(Relation indrel,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
+ olderrinfo = vacrelstats->errinfo;
/* The index name is also saved during this phase */
- vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
- update_vacuum_error_info(vacrelstats,
+ vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel));
+ update_vacuum_error_info(&vacrelstats->errinfo,
VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
InvalidBlockNumber);
*stats = index_vacuum_cleanup(&ivinfo, *stats);
/* Revert back to the old phase information for error traceback */
- update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
- pfree(vacrelstats->indname);
+ update_vacuum_error_info(&vacrelstats->errinfo,
+ olderrinfo.phase,
+ olderrinfo.blkno);
+ pfree(vacrelstats->errinfo.indname);
if (!(*stats))
return;
@@ -2589,7 +2605,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
vacrelstats->lock_waiter_detected = true;
ereport(elevel,
(errmsg("\"%s\": stopping truncate due to conflicting lock request",
- vacrelstats->relname)));
+ vacrelstats->errinfo.relname)));
return;
}
@@ -2622,7 +2638,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
* were vacuuming.
*/
new_rel_pages = count_nondeletable_pages(onerel, vacrelstats);
- vacrelstats->blkno = new_rel_pages;
+ vacrelstats->errinfo.blkno = new_rel_pages;
if (new_rel_pages >= old_rel_pages)
{
@@ -2655,7 +2671,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
ereport(elevel,
(errmsg("\"%s\": truncated %u to %u pages",
- vacrelstats->relname,
+ vacrelstats->errinfo.relname,
old_rel_pages, new_rel_pages),
errdetail_internal("%s",
pg_rusage_show(&ru0))));
@@ -2720,7 +2736,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
{
ereport(elevel,
(errmsg("\"%s\": suspending truncate due to conflicting lock request",
- vacrelstats->relname)));
+ vacrelstats->errinfo.relname)));
vacrelstats->lock_waiter_detected = true;
return blkno;
@@ -3513,13 +3529,13 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
* Initialize vacrelstats for use as error callback arg by parallel
* worker.
*/
- vacrelstats.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
- vacrelstats.relname = pstrdup(RelationGetRelationName(onerel));
- vacrelstats.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
+ vacrelstats.errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+ vacrelstats.errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+ vacrelstats.errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
/* Setup error traceback support for ereport() */
errcallback.callback = vacuum_error_callback;
- errcallback.arg = &vacrelstats;
+ errcallback.arg = &vacrelstats.errinfo;
errcallback.previous = error_context_stack;
error_context_stack = &errcallback;
@@ -3550,7 +3566,7 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
static void
vacuum_error_callback(void *arg)
{
- LVRelStats *errinfo = arg;
+ LVSavedPosition *errinfo = arg;
switch (errinfo->phase)
{
@@ -3591,7 +3607,7 @@ vacuum_error_callback(void *arg)
/* Update vacuum error callback for the current phase and block. */
static void
-update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno)
+update_vacuum_error_info(LVSavedPosition *errinfo, int phase, BlockNumber blkno)
{
errinfo->blkno = blkno;
errinfo->phase = phase;
--
2.17.0
>From 5e15b14c39fcbf9fce1599d0583f1aa568986e43 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 22 Jun 2020 17:13:39 -0500
Subject: [PATCH v2 5/5] Update functions to pass only errinfo struct..
..this is a more complete change, and probably a good idea since parallel
workers have an partially-populated LVRelStats, and it's better to avoid the
idea that it's usable (dead_tuples in particular is a bogus pointer).
---
src/backend/access/heap/vacuumlazy.c | 64 ++++++++++++++--------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 9eb4bc66ae..d239ad4d62 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -344,10 +344,10 @@ static void lazy_vacuum_all_indexes(Relation onerel, Relation *Irel,
LVRelStats *vacrelstats, LVParallelState *lps,
int nindexes);
static void lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
- LVDeadTuples *dead_tuples, double reltuples, LVRelStats *vacrelstats);
+ LVDeadTuples *dead_tuples, double reltuples, LVSavedPosition *errinfo);
static void lazy_cleanup_index(Relation indrel,
IndexBulkDeleteResult **stats,
- double reltuples, bool estimated_count, LVRelStats *vacrelstats);
+ double reltuples, bool estimated_count, LVSavedPosition *errinfo);
static int lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
int tupindex, LVRelStats *vacrelstats, Buffer *vmbuffer);
static bool should_attempt_truncation(VacuumParams *params,
@@ -367,13 +367,13 @@ static void lazy_parallel_vacuum_indexes(Relation *Irel, IndexBulkDeleteResult *
int nindexes);
static void parallel_vacuum_index(Relation *Irel, IndexBulkDeleteResult **stats,
LVShared *lvshared, LVDeadTuples *dead_tuples,
- int nindexes, LVRelStats *vacrelstats);
+ int nindexes, LVSavedPosition *errinfo);
static void vacuum_indexes_leader(Relation *Irel, IndexBulkDeleteResult **stats,
LVRelStats *vacrelstats, LVParallelState *lps,
int nindexes);
static void vacuum_one_index(Relation indrel, IndexBulkDeleteResult **stats,
LVShared *lvshared, LVSharedIndStats *shared_indstats,
- LVDeadTuples *dead_tuples, LVRelStats *vacrelstats);
+ LVDeadTuples *dead_tuples, LVSavedPosition *errinfo);
static void lazy_cleanup_all_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
LVRelStats *vacrelstats, LVParallelState *lps,
int nindexes);
@@ -1797,7 +1797,7 @@ lazy_vacuum_all_indexes(Relation onerel, Relation *Irel,
for (idx = 0; idx < nindexes; idx++)
lazy_vacuum_index(Irel[idx], &stats[idx], vacrelstats->dead_tuples,
- vacrelstats->old_live_tuples, vacrelstats);
+ vacrelstats->old_live_tuples, &vacrelstats->errinfo);
}
/* Increase and report the number of index scans */
@@ -2160,7 +2160,7 @@ lazy_parallel_vacuum_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
* indexes in the case where no workers are launched.
*/
parallel_vacuum_index(Irel, stats, lps->lvshared,
- vacrelstats->dead_tuples, nindexes, vacrelstats);
+ vacrelstats->dead_tuples, nindexes, &vacrelstats->errinfo);
/*
* Next, accumulate buffer and WAL usage. (This must wait for the workers
@@ -2195,7 +2195,7 @@ lazy_parallel_vacuum_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
static void
parallel_vacuum_index(Relation *Irel, IndexBulkDeleteResult **stats,
LVShared *lvshared, LVDeadTuples *dead_tuples,
- int nindexes, LVRelStats *vacrelstats)
+ int nindexes, LVSavedPosition *errinfo)
{
/*
* Increment the active worker count if we are able to launch any worker.
@@ -2229,7 +2229,7 @@ parallel_vacuum_index(Relation *Irel, IndexBulkDeleteResult **stats,
/* Do vacuum or cleanup of the index */
vacuum_one_index(Irel[idx], &(stats[idx]), lvshared, shared_indstats,
- dead_tuples, vacrelstats);
+ dead_tuples, errinfo);
}
/*
@@ -2270,7 +2270,7 @@ vacuum_indexes_leader(Relation *Irel, IndexBulkDeleteResult **stats,
skip_parallel_vacuum_index(Irel[i], lps->lvshared))
vacuum_one_index(Irel[i], &(stats[i]), lps->lvshared,
shared_indstats, vacrelstats->dead_tuples,
- vacrelstats);
+ &vacrelstats->errinfo);
}
/*
@@ -2290,7 +2290,7 @@ vacuum_indexes_leader(Relation *Irel, IndexBulkDeleteResult **stats,
static void
vacuum_one_index(Relation indrel, IndexBulkDeleteResult **stats,
LVShared *lvshared, LVSharedIndStats *shared_indstats,
- LVDeadTuples *dead_tuples, LVRelStats *vacrelstats)
+ LVDeadTuples *dead_tuples, LVSavedPosition *errinfo)
{
IndexBulkDeleteResult *bulkdelete_res = NULL;
@@ -2310,10 +2310,10 @@ vacuum_one_index(Relation indrel, IndexBulkDeleteResult **stats,
/* Do vacuum or cleanup of the index */
if (lvshared->for_cleanup)
lazy_cleanup_index(indrel, stats, lvshared->reltuples,
- lvshared->estimated_count, vacrelstats);
+ lvshared->estimated_count, errinfo);
else
lazy_vacuum_index(indrel, stats, dead_tuples,
- lvshared->reltuples, vacrelstats);
+ lvshared->reltuples, errinfo);
/*
* Copy the index bulk-deletion result returned from ambulkdelete and
@@ -2389,7 +2389,7 @@ lazy_cleanup_all_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
lazy_cleanup_index(Irel[idx], &stats[idx],
vacrelstats->new_rel_tuples,
vacrelstats->tupcount_pages < vacrelstats->rel_pages,
- vacrelstats);
+ &vacrelstats->errinfo);
}
}
@@ -2404,7 +2404,7 @@ lazy_cleanup_all_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
*/
static void
lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
- LVDeadTuples *dead_tuples, double reltuples, LVRelStats *vacrelstats)
+ LVDeadTuples *dead_tuples, double reltuples, LVSavedPosition *errinfo)
{
IndexVacuumInfo ivinfo;
const char *msg;
@@ -2422,10 +2422,10 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
- olderrinfo = vacrelstats->errinfo;
+ olderrinfo = *errinfo;
/* The index name is also saved during this phase */
- vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel));
- update_vacuum_error_info(&vacrelstats->errinfo,
+ errinfo->indname = pstrdup(RelationGetRelationName(indrel));
+ update_vacuum_error_info(errinfo,
VACUUM_ERRCB_PHASE_VACUUM_INDEX,
InvalidBlockNumber);
@@ -2440,15 +2440,15 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ereport(elevel,
(errmsg(msg,
- vacrelstats->errinfo.indname,
+ errinfo->indname,
dead_tuples->num_tuples),
errdetail_internal("%s", pg_rusage_show(&ru0))));
/* Revert to the previous phase information for error traceback */
- update_vacuum_error_info(&vacrelstats->errinfo,
+ update_vacuum_error_info(errinfo,
olderrinfo.phase,
olderrinfo.blkno);
- pfree(vacrelstats->errinfo.indname);
+ pfree(errinfo->indname);
}
/*
@@ -2460,7 +2460,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
static void
lazy_cleanup_index(Relation indrel,
IndexBulkDeleteResult **stats,
- double reltuples, bool estimated_count, LVRelStats *vacrelstats)
+ double reltuples, bool estimated_count, LVSavedPosition *errinfo)
{
IndexVacuumInfo ivinfo;
const char *msg;
@@ -2479,20 +2479,20 @@ lazy_cleanup_index(Relation indrel,
ivinfo.strategy = vac_strategy;
/* Update error traceback information */
- olderrinfo = vacrelstats->errinfo;
+ olderrinfo = *errinfo;
/* The index name is also saved during this phase */
- vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel));
- update_vacuum_error_info(&vacrelstats->errinfo,
+ errinfo->indname = pstrdup(RelationGetRelationName(indrel));
+ update_vacuum_error_info(errinfo,
VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
InvalidBlockNumber);
*stats = index_vacuum_cleanup(&ivinfo, *stats);
/* Revert back to the old phase information for error traceback */
- update_vacuum_error_info(&vacrelstats->errinfo,
+ update_vacuum_error_info(errinfo,
olderrinfo.phase,
olderrinfo.blkno);
- pfree(vacrelstats->errinfo.indname);
+ pfree(errinfo->indname);
if (!(*stats))
return;
@@ -3474,7 +3474,7 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
int nindexes;
char *sharedquery;
IndexBulkDeleteResult **stats;
- LVRelStats vacrelstats;
+ LVSavedPosition errinfo;
ErrorContextCallback errcallback;
lvshared = (LVShared *) shm_toc_lookup(toc, PARALLEL_VACUUM_KEY_SHARED,
@@ -3529,13 +3529,13 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
* Initialize vacrelstats for use as error callback arg by parallel
* worker.
*/
- vacrelstats.errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
- vacrelstats.errinfo.relname = pstrdup(RelationGetRelationName(onerel));
- vacrelstats.errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
+ errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+ errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+ errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
/* Setup error traceback support for ereport() */
errcallback.callback = vacuum_error_callback;
- errcallback.arg = &vacrelstats.errinfo;
+ errcallback.arg = &errinfo;
errcallback.previous = error_context_stack;
error_context_stack = &errcallback;
@@ -3544,7 +3544,7 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
/* Process indexes to perform vacuum/cleanup */
parallel_vacuum_index(indrels, stats, lvshared, dead_tuples, nindexes,
- &vacrelstats);
+ &errinfo);
/* Report buffer/WAL usage during parallel execution */
buffer_usage = shm_toc_lookup(toc, PARALLEL_VACUUM_KEY_BUFFER_USAGE, false);
--
2.17.0
