On 2019-Dec-11, Justin Pryzby wrote: > @@ -635,6 +644,15 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, > LVRelStats *vacrelstats, > else > skipping_blocks = false; > > + /* Setup error traceback support for ereport() */ > + errcallback.callback = vacuum_error_callback; > + cbarg.relname = relname; > + cbarg.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); > + cbarg.blkno = 0; /* Not known yet */
Shouldn't you use InvalidBlockNumber for this initialization? > @@ -658,6 +676,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, > LVRelStats *vacrelstats, > > pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, > blkno); > > + cbarg.blkno = blkno; I would put this before pgstat_progress_update_param, just out of paranoia. > @@ -817,7 +837,6 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, > LVRelStats *vacrelstats, > > buf = ReadBufferExtended(onerel, MAIN_FORKNUM, blkno, > RBM_NORMAL, > vac_strategy); > - > /* We need buffer cleanup lock so that we can prune HOT chains. > */ > if (!ConditionalLockBufferForCleanup(buf)) > { Lose this hunk? > @@ -2354,3 +2376,15 @@ 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); > +} I would put this function around line 1512 (just after lazy_scan_heap) rather than at bottom of file. (And move its prototype accordingly, to line 156.) Or do you intend that this is going to be used for lazy_vacuum_heap too? Maybe it should. Patch looks good to me otherwise. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services