From: Justin Pryzby <pryzby@telsasoft.com>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: error context for vacuum to include block number
Date: Thu, 13 Feb 2020 17:52:54 -0600
Message-ID: <20200213235254.GC31889@telsasoft.com> (raw)
In-Reply-To: <CA+fd4k4QHeO_fpFjYGd4RRxMWtNWwXZozG2JGTWdo8YRQ15k8g@mail.gmail.com>
References: <20200120214929.GV26045@telsasoft.com>
<20200126202938.dqtavh6vr3vgpw53@alap3.anarazel.de>
<20200127053813.GS13621@telsasoft.com>
<CA+fd4k71fHRYJAeGxigGza5U+_HDJcBAbMniiMK2xz5GHhsMmQ@mail.gmail.com>
<20200127225018.GY13621@telsasoft.com>
<CA+fd4k48wcdYpr-4UsLzchohOk6pwW3Aijmw_PR4hEXJBcs8Xw@mail.gmail.com>
<20200202060259.GG13621@telsasoft.com>
<CA+fd4k7U__9nWB5q0njLc66rBJfekKAUVJXOPJ0HEw-DYocyLQ@mail.gmail.com>
<20200208010107.GT403@telsasoft.com>
<CA+fd4k4QHeO_fpFjYGd4RRxMWtNWwXZozG2JGTWdo8YRQ15k8g@mail.gmail.com>
On Thu, Feb 13, 2020 at 02:55:53PM +0900, Masahiko Sawada wrote:
> You need to add a newline to follow the limit line lengths so that the
> code is readable in an 80-column window. Or please run pgindent.
For now I :set tw=80
> 2.
> I think that making initialization process of errcontext argument a
> function is good. But maybe we can merge these two functions into one.
Thanks, this is better, and I used that.
> init_error_context_heap and init_error_context_index actually don't
> only initialize the callback arguments but also push the vacuum
> errcallback, in spite of the function name having 'init'. Also I think
> it might be better to only initialize the callback arguments in this
> function and to set errcallback by caller, rather than to wrap pushing
> errcallback by a function.
However I think it's important not to repeat this 4 times:
errcallback->callback = vacuum_error_callback;
errcallback->arg = errcbarg;
errcallback->previous = error_context_stack;
error_context_stack = errcallback;
So I kept the first 3 of those in the function and copied only assignment to
the global. That helps makes the heap scan function clear, which assigns to it
twice.
BTW, for testing, I'm able to consistently hit the "vacuuming block" case like
this:
SET statement_timeout=0; DROP TABLE t; CREATE TABLE t(i int); CREATE INDEX ON t(i); INSERT INTO t SELECT generate_series(1,99999); UPDATE t SET i=i-1; SET statement_timeout=111; SET vacuum_cost_delay=3; SET vacuum_cost_page_dirty=0; SET vacuum_cost_page_hit=11; SET vacuum_cost_limit=33; SET statement_timeout=3333; VACUUM VERBOSE t;
Thanks for re-reviewing.
--
Justin
Attachments:
[text/x-diff] v18-0001-vacuum-errcontext-to-show-block-being-processed.patch (8.6K, ../20200213235254.GC31889@telsasoft.com/2-v18-0001-vacuum-errcontext-to-show-block-being-processed.patch)
download | inline diff:
From 5b8cad37244cdc310d78719b64ff44a464910598 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 12 Dec 2019 20:54:37 -0600
Subject: [PATCH v18] vacuum errcontext to show block being processed
Discussion:
https://www.postgresql.org/message-id/20191120210600.GC30362@telsasoft.com
---
src/backend/access/heap/vacuumlazy.c | 120 +++++++++++++++++++++++++++++++++++
1 file changed, 120 insertions(+)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.cindex a23cdef..209f483 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,9 @@ 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);+static void init_error_context(ErrorContextCallback *errcallback,+ vacuum_error_callback_arg *errcbarg, Relation onerel, int phase);
/*
@@ -724,6 +735,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 +883,10 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
else
skipping_blocks = false;
+ /* Setup error traceback support for ereport() */+ init_error_context(&errcallback, &errcbarg, onerel, PROGRESS_VACUUM_PHASE_SCAN_HEAP);+ error_context_stack = &errcallback;+
for (blkno = 0; blkno < nblocks; blkno++)
{
Buffer buf;
@@ -891,6 +908,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 +1006,9 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
vmbuffer = InvalidBuffer;
}
+ /* Pop the error context stack while calling vacuum */+ 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 +1033,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 +1622,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 +1800,19 @@ 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()+ */+ init_error_context(&errcallback, &errcbarg, onerel, PROGRESS_VACUUM_PHASE_VACUUM_HEAP);+ error_context_stack = &errcallback;+
pg_rusage_init(&ru0);
npages = 0;
@@ -1791,6 +1827,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 +1848,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 +2358,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 +2371,17 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
ivinfo.num_heap_tuples = reltuples;
ivinfo.strategy = vac_strategy;
+ /* Setup error traceback support for ereport() */+ init_error_context(&errcallback, &errcbarg, indrel, PROGRESS_VACUUM_PHASE_VACUUM_INDEX);+ 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 +2408,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 +2422,15 @@ lazy_cleanup_index(Relation indrel,
ivinfo.num_heap_tuples = reltuples;
ivinfo.strategy = vac_strategy;
+ /* Setup error traceback support for ereport() */+ init_error_context(&errcallback, &errcbarg, indrel, PROGRESS_VACUUM_PHASE_INDEX_CLEANUP);+ 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 +3433,65 @@ 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;+ }+}++/* Initialize error context for heap operations */+static void+init_error_context(ErrorContextCallback *errcallback, vacuum_error_callback_arg *errcbarg, Relation rel, int phase)+{+ switch (phase)+ {+ case PROGRESS_VACUUM_PHASE_SCAN_HEAP:+ case PROGRESS_VACUUM_PHASE_VACUUM_HEAP:+ errcbarg->relname = RelationGetRelationName(rel);+ errcbarg->indname = NULL; /* Not used for heap */+ break;++ case PROGRESS_VACUUM_PHASE_VACUUM_INDEX:+ case PROGRESS_VACUUM_PHASE_INDEX_CLEANUP:+ /* indname is the index being processed, relname is its relation */+ errcbarg->indname = RelationGetRelationName(rel);+ errcbarg->relname = get_rel_name(rel->rd_index->indexrelid);+ break;+ }++ errcbarg->relnamespace = get_namespace_name(RelationGetNamespace(rel));+ errcbarg->blkno = InvalidBlockNumber; /* Not known yet */+ errcbarg->phase = phase;++ errcallback->callback = vacuum_error_callback;+ errcallback->arg = errcbarg;+ errcallback->previous = error_context_stack;+}--
2.7.4
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: pryzby@telsasoft.com, masahiko.sawada@2ndquadrant.com, andres@anarazel.de, michael@paquier.xyz, alvherre@2ndquadrant.com
Subject: Re: error context for vacuum to include block number
In-Reply-To: <20200213235254.GC31889@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox