On Tue, Mar 24, 2020 at 7:51 PM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
>
> On Tue, 24 Mar 2020 at 22:37, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, Mar 24, 2020 at 6:18 PM Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> > >
> > >
> > > I've read through the latest version patch (v31), here are my comments:
> > >
> > > 1.
> > > + /* Update error traceback information */
> > > + olderrcbarg = *vacrelstats;
> > > + update_vacuum_error_cbarg(vacrelstats,
> > > + VACUUM_ERRCB_PHASE_TRUNCATE,
> > > new_rel_pages, NULL,
> > > + false);
> > > +
> > > /*
> > > * Scan backwards from the end to verify that the end pages actually
> > > * contain no tuples. This is *necessary*, not optional, because
> > > * other backends could have added tuples to these pages whilst we
> > > * were vacuuming.
> > > */
> > > new_rel_pages = count_nondeletable_pages(onerel, vacrelstats);
> > >
> > > We need to set the error context after setting new_rel_pages.
> > >
> >
> > We want to cover the errors raised in count_nondeletable_pages(). In
> > an earlier version of the patch, we had TRUNCATE_PREFETCH phase which
> > use to cover those errors, but that was not good as we were
> > setting/resetting it multiple times and it was not clear such a
> > separate phase would add any value.
>
> I got the point. But if we set the error context before that, I think
> we need to change the error context message. The error context message
> of heap truncation phase is "while truncating relation \"%s.%s\" to %u
> blocks", but cbarg->blkno will be the number of blocks of the current
> relation.
>
> case VACUUM_ERRCB_PHASE_TRUNCATE:
> if (BlockNumberIsValid(cbarg->blkno))
> errcontext("while truncating relation \"%s.%s\" to %u blocks",
> cbarg->relnamespace, cbarg->relname, cbarg->blkno);
> break;
>
Do you mean to say that actually we are just prefetching or reading
the pages in count_nondeletable_pages() but the message doesn't have
any such indication? If not that, what problem do you see with the
message? What is your suggestion?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
