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 04:52:21
Message-ID: CAA4eK1L3R5_1vse+ceCYwiMYCeKgC1f+9mQ4jSmzhnkGKz5SOA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 25, 2020 at 10:05 AM Masahiko Sawada
<masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
>
> On Wed, 25 Mar 2020 at 12:44, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Tue, Mar 24, 2020 at 7:51 PM Masahiko Sawada
> > <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > >
> > >
> > > 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?
>
> I meant that with the patch, suppose that the table has 100 blocks and
> we're truncating it to 50 blocks in RelationTruncate(), the error
> context message will be "while truncating relation "aaa.bbb" to 100
> blocks", which is not correct. I think it should be "while truncating
> relation "aaa.bbb" to 50 blocks". We can know the relation can be
> truncated to 50 blocks by the result of count_nondeletable_pages(). So
> if we update the arguments before it we will use the number of blocks
> of relation before truncation.
>

Won't the latest patch by Justin will fix this as he has updated the
block count after count_nondeletable_pages? Apart from that, I feel
the first call to update_vacuum_error_cbarg in lazy_truncate_heap
should have input parameter as vacrelstats->nonempty_pages instead of
new_rel_pages to indicate the remaining pages after truncation?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-03-25 05:03:14 Re: bad logging around broken restore_command
Previous Message Justin Pryzby 2020-03-25 04:46:51 Re: error context for vacuum to include block number