Re: error context for vacuum to include block number

Hi,

On 2020-01-22 17:17:26 -0600, Justin Pryzby wrote:
> On Mon, Jan 20, 2020 at 11:11:20AM -0800, Andres Freund wrote:
> > > @@ -966,8 +986,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, Irel, indstats,
> > > vacrelstats, lps, nindexes);
> > > + error_context_stack = &errcallback;
> >
> > Alternatively we could push another context for each index inside
> > lazy_vacuum_all_indexes(). There's been plenty bugs in indexes
> > triggering problems, so that could be worthwhile.
>
> Is the callback for index vacuum useful without a block number?

Yea, it is. Without at least that context I don't think we even will
reliably know which index we're dealing with in case of an error.

> FYI, I have another patch which would add DEBUG output before each stage, which
> would be just as much information, and without needing to use a callback.
> It's 0004 here:

I don't think that is equivalent at all. With a context I see the
context in the log in case of an error. With a DEBUG message I need to
be able to reproduce the error (without even knowing which relation
etc).

Greetings,

Andres Freund