Re: error context for vacuum to include block number

On Fri, Mar 27, 2020 at 3:47 AM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
>
>
> > Hm, I was just wondering what happens if an error happens *during*
> > update_vacuum_error_cbarg(). It seems like if we set
> > errcbarg->phase=VACUUM_INDEX before setting errcbarg->indname=indname, then an
> > error would cause a crash.
> >

Can't that be avoided if you check if cbarg->indname is non-null in
vacuum_error_callback as we are already doing for
VACUUM_ERRCB_PHASE_TRUNCATE?

> > And if we pfree and set indname before phase, it'd
> > be a problem when going from an index phase to non-index phase.

How is it possible that we move to the non-index phase without
clearing indname as we always revert back the old phase information?
I think it is possible only if we don't clear indname after the phase
is over.

> > So maybe we
> > have to set errcbarg->phase=VACUUM_ERRCB_PHASE_UNKNOWN while in the function,
> > and errcbarg->phase=phase last.

I find that a bit ad-hoc, if possible, let's try to avoid it.

>
> And addressed that.
>
> Also, I realized that lazy_cleanup_index has an early "return", so the "Revert
> back" was ineffective.
>

We can call it immediately after index_vacuum_cleanup to avoid that.

> We talked about how that wasn't needed, since we never
> go back to a previous phase. Amit wanted to keep it there for consistency, but
> I'd prefer to put any extra effort into calling out the special treatment
> needed/given to lazy_vacuum_heap/index, rather than making everything
> "consistent".
>

Apart from being consistent, the point was it doesn't seem good that
API being called to assume that there is nothing more the caller can
do. It might be problematic if we later want to enhance or add
something to the caller.

> Amit: I also moved the TRUNCATE_HEAP bit back to truncate_heap(),

There is no problem with it. We can do it either way and I have also
considered it the way you have done but decide to keep in the caller
because of the previous point I mentioned (not sure if it a good idea
that API being called can assume that there is nothing more the caller
can do after this).

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com