From: Justin Pryzby <pryzby@telsasoft.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: error context for vacuum to include block number
Date: Wed, 25 Mar 2020 00:08:19 -0500
Message-ID: <20200325050819.GP21443@telsasoft.com> (raw)
In-Reply-To: <CAA4eK1L3R5_1vse+ceCYwiMYCeKgC1f+9mQ4jSmzhnkGKz5SOA@mail.gmail.com>
References: <20200324041616.GA21443@telsasoft.com>
<CAA4eK1J4wNO425WjKj2Yd58Ag=LovSkxoHTuBJphFma+9jLAJA@mail.gmail.com>
<CA+fd4k5TFS87WXsgne17dbYGUq79bvx-WnumfF22eqkjCHtXkA@mail.gmail.com>
<CAA4eK1+vjO-M4OaHUjjDpTkR+g47BC+mSr64qniZac6Qks_w_w@mail.gmail.com>
<CA+fd4k4ayy53qhn=rDNvazM8-rNOQ5wsAuZP05ekvvwsaed4Wg@mail.gmail.com>
<CAA4eK1+O8q9_e10DTOnAbX6Mf9C1bSDBZFmhCLSihb-ZgBXsGQ@mail.gmail.com>
<CA+fd4k4LfJwv=JaLFG4pZKvwwimO6MHFZ7YfMFqubdKXjiRDVw@mail.gmail.com>
<CAA4eK1+v7nKwX8oNwfTur3EKW8QDWbgFYzyjsHtRQWDqazq2zA@mail.gmail.com>
<CA+fd4k5e1q4t9k4pLQah1qx8GZKy2wb1EUp_YsNOGfgOxX+GHg@mail.gmail.com>
<CAA4eK1L3R5_1vse+ceCYwiMYCeKgC1f+9mQ4jSmzhnkGKz5SOA@mail.gmail.com>
On Wed, Mar 25, 2020 at 10:22:21AM +0530, Amit Kapila wrote:
> On Wed, Mar 25, 2020 at 10:05 AM Masahiko Sawada
> <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > On Wed, 25 Mar 2020 at 12:44, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, Mar 24, 2020 at 7:51 PM Masahiko Sawada
> > > <masahiko.sawada@2ndquadrant.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 issue is if the error happens *during* count_nondeletable_pages().
We don't want it to say "truncating relation to 100 blocks".
> 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?
Yea, I think that addresses the issue.
--
Justin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, amit.kapila16@gmail.com, masahiko.sawada@2ndquadrant.com, alvherre@2ndquadrant.com, andres@anarazel.de, michael@paquier.xyz
Subject: Re: error context for vacuum to include block number
In-Reply-To: <20200325050819.GP21443@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox