Re: error context for vacuum to include block number

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: error context for vacuum to include block number
Date: 2020-03-25 03:44:42
Message-ID: CAA4eK1+v7nKwX8oNwfTur3EKW8QDWbgFYzyjsHtRQWDqazq2zA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 24, 2020 at 7:51 PM Masahiko Sawada
<masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
>
> On Tue, 24 Mar 2020 at 22:37, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Tue, Mar 24, 2020 at 6:18 PM Masahiko Sawada
> > <masahiko(dot)sawada(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-03-25 03:46:46 Re: error context for vacuum to include block number
Previous Message Justin Pryzby 2020-03-25 03:19:22 Re: error context for vacuum to include block number