On Tue, Feb 04, 2020 at 01:58:20PM +0900, Masahiko Sawada wrote: > Here is the comment for v16 patch: > > 2. > I think we can set the error context for heap scan again after > freespace map vacuum finishing, maybe after reporting the new phase. > Otherwise the user will get confused if an error occurs during > freespace map vacuum. And I think the comment is unclear, how about > "Set the error context fro heap scan again"?
Good point > 3. > + if (cbarg->blkno!=InvalidBlockNumber) > + errcontext(_("while scanning block %u of relation \"%s.%s\""), > + cbarg->blkno, cbarg->relnamespace, cbarg->relname); > > We can use BlockNumberIsValid macro instead. Thanks. See attached, now squished together patches. I added functions to initialize the callbacks, so error handling is out of the way and minimally distract from the rest of vacuum.
>From 95265412c56f3b308eed16531d7c83243e278f4f Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Thu, 12 Dec 2019 20:54:37 -0600 Subject: [PATCH v17 1/2] 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 | 117 +++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index a23cdef..9358ab4 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -292,6 +292,14 @@ typedef struct LVRelStats bool lock_waiter_detected; } LVRelStats; +typedef struct +{ + char *relnamespace; + char *relname; + char *indname; /* undefined while not processing index */ + BlockNumber blkno; /* undefined while not processing heap */ + int phase; /* Reusing same enums as for progress reporting */ +} vacuum_error_callback_arg; /* A few variables that don't seem worth passing around as parameters */ static int elevel = -1; @@ -361,6 +369,7 @@ static void end_parallel_vacuum(Relation *Irel, IndexBulkDeleteResult **stats, LVParallelState *lps, int nindexes); 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); /* @@ -724,6 +733,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); @@ -870,6 +881,17 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, else skipping_blocks = false; + /* Setup error traceback support for ereport() */ + errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); + errcbarg.relname = relname; + errcbarg.blkno = InvalidBlockNumber; /* Not known yet */ + errcbarg.phase = PROGRESS_VACUUM_PHASE_SCAN_HEAP; + + 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; @@ -891,6 +913,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) @@ -987,6 +1011,9 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, vmbuffer = InvalidBuffer; } + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + /* Work on all the indexes, then the heap */ lazy_vacuum_all_indexes(onerel, Irel, indstats, vacrelstats, lps, nindexes); @@ -1011,6 +1038,9 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, /* Report that we are once again scanning the heap */ pgstat_progress_update_param(PROGRESS_VACUUM_PHASE, PROGRESS_VACUUM_PHASE_SCAN_HEAP); + + /* Set the error context while continuing heap scan */ + error_context_stack = &errcallback; } /* @@ -1597,6 +1627,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); @@ -1772,11 +1805,26 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) int npages; 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 */ + errcbarg.phase = PROGRESS_VACUUM_PHASE_VACUUM_HEAP; + + errcallback.callback = vacuum_error_callback; + errcallback.arg = (void *) &errcbarg; + errcallback.previous = error_context_stack; + error_context_stack = &errcallback; + pg_rusage_init(&ru0); npages = 0; @@ -1791,6 +1839,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) vacuum_delay_point(); tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]); + errcbarg.blkno = tblk; buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL, vac_strategy); if (!ConditionalLockBufferForCleanup(buf)) @@ -1811,6 +1860,9 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) npages++; } + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + if (BufferIsValid(vmbuffer)) { ReleaseBuffer(vmbuffer); @@ -2318,6 +2370,8 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, IndexVacuumInfo ivinfo; const char *msg; PGRUsage ru0; + ErrorContextCallback errcallback; + vacuum_error_callback_arg errcbarg; pg_rusage_init(&ru0); @@ -2329,10 +2383,24 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, ivinfo.num_heap_tuples = reltuples; ivinfo.strategy = vac_strategy; + /* Setup error traceback support for ereport() */ + errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(indrel)); + errcbarg.indname = RelationGetRelationName(indrel); + errcbarg.relname = get_rel_name(indrel->rd_index->indexrelid); + errcbarg.phase = PROGRESS_VACUUM_PHASE_VACUUM_INDEX; + + errcallback.callback = vacuum_error_callback; + errcallback.arg = (void *) &errcbarg; + errcallback.previous = error_context_stack; + error_context_stack = &errcallback; + /* Do bulk deletion */ *stats = index_bulk_delete(&ivinfo, *stats, lazy_tid_reaped, (void *) dead_tuples); + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + if (IsParallelWorker()) msg = gettext_noop("scanned index \"%s\" to remove %d row versions by parallel vacuum worker"); else @@ -2359,6 +2427,8 @@ lazy_cleanup_index(Relation indrel, IndexVacuumInfo ivinfo; const char *msg; PGRUsage ru0; + vacuum_error_callback_arg errcbarg; + ErrorContextCallback errcallback; pg_rusage_init(&ru0); @@ -2371,8 +2441,22 @@ lazy_cleanup_index(Relation indrel, ivinfo.num_heap_tuples = reltuples; ivinfo.strategy = vac_strategy; + /* Setup error traceback support for ereport() */ + errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(indrel)); + errcbarg.indname = RelationGetRelationName(indrel); + errcbarg.relname = get_rel_name(indrel->rd_index->indexrelid); + errcbarg.phase = PROGRESS_VACUUM_PHASE_INDEX_CLEANUP; + + errcallback.previous = error_context_stack; + errcallback.callback = vacuum_error_callback; + errcallback.arg = (void *) &errcbarg; + error_context_stack = &errcallback; + *stats = index_vacuum_cleanup(&ivinfo, *stats); + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + if (!(*stats)) return; @@ -3375,3 +3459,36 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) table_close(onerel, ShareUpdateExclusiveLock); pfree(stats); } + +/* + * Error context callback for errors occurring during vacuum. + */ +static void +vacuum_error_callback(void *arg) +{ + vacuum_error_callback_arg *cbarg = arg; + + switch (cbarg->phase) { + case PROGRESS_VACUUM_PHASE_SCAN_HEAP: + if (BlockNumberIsValid(cbarg->blkno)) + errcontext(_("while scanning block %u of relation \"%s.%s\""), + cbarg->blkno, cbarg->relnamespace, cbarg->relname); + break; + + case PROGRESS_VACUUM_PHASE_VACUUM_HEAP: + if (BlockNumberIsValid(cbarg->blkno)) + errcontext(_("while vacuuming block %u of relation \"%s.%s\""), + cbarg->blkno, cbarg->relnamespace, cbarg->relname); + break; + + case PROGRESS_VACUUM_PHASE_VACUUM_INDEX: + errcontext(_("while vacuuming index \"%s.%s\" of relation \"%s\""), + cbarg->relnamespace, cbarg->indname, cbarg->relname); + break; + + case PROGRESS_VACUUM_PHASE_INDEX_CLEANUP: + errcontext(_("while cleaning up index \"%s.%s\" of relation \"%s\""), + cbarg->relnamespace, cbarg->indname, cbarg->relname); + break; + } +} -- 2.7.4
>From 410fcd0106c5ba9a4718b79d2a758fa40c246afc Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Thu, 6 Feb 2020 10:27:05 -0600 Subject: [PATCH v17 2/2] Functions to initialize errcontext --- src/backend/access/heap/vacuumlazy.c | 68 +++++++++++++++++------------------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 9358ab4..361e595 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -370,6 +370,8 @@ 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 init_error_context_heap(ErrorContextCallback *errcallback, vacuum_error_callback_arg *errcbarg, Relation onerel, int phase); +static void init_error_context_index(ErrorContextCallback *errcallback, vacuum_error_callback_arg *errcbarg, Relation indrel, int phase); /* @@ -882,15 +884,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, skipping_blocks = false; /* Setup error traceback support for ereport() */ - errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); - errcbarg.relname = relname; - errcbarg.blkno = InvalidBlockNumber; /* Not known yet */ - errcbarg.phase = PROGRESS_VACUUM_PHASE_SCAN_HEAP; - - errcallback.callback = vacuum_error_callback; - errcallback.arg = (void *) &errcbarg; - errcallback.previous = error_context_stack; - error_context_stack = &errcallback; + init_error_context_heap(&errcallback, &errcbarg, onerel, PROGRESS_VACUUM_PHASE_SCAN_HEAP); for (blkno = 0; blkno < nblocks; blkno++) { @@ -1815,15 +1809,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) /* * Setup error traceback support for ereport() */ - errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); - errcbarg.relname = RelationGetRelationName(onerel); - errcbarg.blkno = InvalidBlockNumber; /* Not known yet */ - errcbarg.phase = PROGRESS_VACUUM_PHASE_VACUUM_HEAP; - - errcallback.callback = vacuum_error_callback; - errcallback.arg = (void *) &errcbarg; - errcallback.previous = error_context_stack; - error_context_stack = &errcallback; + init_error_context_heap(&errcallback, &errcbarg, onerel, PROGRESS_VACUUM_PHASE_VACUUM_HEAP); pg_rusage_init(&ru0); npages = 0; @@ -2384,15 +2370,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, ivinfo.strategy = vac_strategy; /* Setup error traceback support for ereport() */ - errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(indrel)); - errcbarg.indname = RelationGetRelationName(indrel); - errcbarg.relname = get_rel_name(indrel->rd_index->indexrelid); - errcbarg.phase = PROGRESS_VACUUM_PHASE_VACUUM_INDEX; - - errcallback.callback = vacuum_error_callback; - errcallback.arg = (void *) &errcbarg; - errcallback.previous = error_context_stack; - error_context_stack = &errcallback; + init_error_context_index(&errcallback, &errcbarg, indrel, PROGRESS_VACUUM_PHASE_VACUUM_INDEX); /* Do bulk deletion */ *stats = index_bulk_delete(&ivinfo, *stats, @@ -2442,15 +2420,7 @@ lazy_cleanup_index(Relation indrel, ivinfo.strategy = vac_strategy; /* Setup error traceback support for ereport() */ - errcbarg.relnamespace = get_namespace_name(RelationGetNamespace(indrel)); - errcbarg.indname = RelationGetRelationName(indrel); - errcbarg.relname = get_rel_name(indrel->rd_index->indexrelid); - errcbarg.phase = PROGRESS_VACUUM_PHASE_INDEX_CLEANUP; - - errcallback.previous = error_context_stack; - errcallback.callback = vacuum_error_callback; - errcallback.arg = (void *) &errcbarg; - error_context_stack = &errcallback; + init_error_context_index(&errcallback, &errcbarg, indrel, PROGRESS_VACUUM_PHASE_INDEX_CLEANUP); *stats = index_vacuum_cleanup(&ivinfo, *stats); @@ -3492,3 +3462,29 @@ vacuum_error_callback(void *arg) break; } } + +/* Initialize error context for heap operations */ +static void +init_error_context_heap(ErrorContextCallback *errcallback, vacuum_error_callback_arg *errcbarg, Relation onerel, int phase) +{ + errcbarg->relnamespace = get_namespace_name(RelationGetNamespace(onerel)); + errcbarg->relname = RelationGetRelationName(onerel); + errcbarg->indname = NULL; /* Not used for heap */ + errcbarg->blkno = InvalidBlockNumber; /* Not known yet */ + errcbarg->phase = phase; + + errcallback->callback = vacuum_error_callback; + errcallback->arg = errcbarg; + errcallback->previous = error_context_stack; + error_context_stack = errcallback; +} + +/* Initialize error context for index operations */ +static void +init_error_context_index(ErrorContextCallback *errcallback, vacuum_error_callback_arg *errcbarg, Relation indrel, int phase) +{ + /* Piggyback on the heap function but specially set relation names. Note, blkno is not used for index. */ + init_error_context_heap(errcallback, errcbarg, indrel, phase); + errcbarg->indname = RelationGetRelationName(indrel); + errcbarg->relname = get_rel_name(indrel->rd_index->indexrelid); +} -- 2.7.4