RE: [PATCH] Speedup truncates of relation forks - Mailing list pgsql-hackers

From Jamison, Kirk
Subject RE: [PATCH] Speedup truncates of relation forks
Date
Msg-id D09B13F772D2274BB348A310EE3027C6590FF6@g01jpexmbkw24
Whole thread Raw
In response to Re: [PATCH] Speedup truncates of relation forks  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: [PATCH] Speedup truncates of relation forks
Re: [PATCH] Speedup truncates of relation forks
List pgsql-hackers
On Friday, September 13, 2019 10:06 PM (GMT+9), Fujii Masao wrote:
> On Fri, Sep 13, 2019 at 9:51 PM Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:
> >
> > On 2019-Sep-13, Fujii Masao wrote:
> >
> > > On Mon, Sep 9, 2019 at 3:52 PM Jamison, Kirk <k.jamison@jp.fujitsu.com>
> wrote:
> >
> > > > > Please add a preliminary patch that removes the function.  Dead
> > > > > code is good, as long as it is gone.  We can get it pushed ahead of
> the rest of this.
> > > >
> > > > Alright. I've attached a separate patch removing the smgrdounlinkfork.
> > >
> > > Per the past discussion, some people want to keep this "dead"
> > > function for some reasons. So, in my opinion, it's better to just
> > > enclose the function with #if NOT_USED and #endif, to keep the
> > > function itself as it is, and then to start new discussion on
> > > hackers about the removal of that separatedly from this patch.
> >
> > I searched for anybody requesting to keep the function.  I couldn't
> > find anything.  Tom said in 2012:
> > https://www.postgresql.org/message-id/1471.1339106082@sss.pgh.pa.us
> 
> Yes. And I found Andres.
> https://www.postgresql.org/message-id/20180621174129.hogefyopje4xaznu@al
> ap3.anarazel.de
> 
> > > As committed, the smgrdounlinkfork case is actually dead code; it's
> > > never called from anywhere.  I left it in place just in case we want
> > > it someday.
> >
> > but if no use has appeared in 7 years, I say it's time to kill it.
> 
> +1

The consensus is we remove it, right?
Re-attaching the patch that removes the deadcode: smgrdounlinkfork().

---
I've also fixed Fujii-san's comments below in the latest attached speedup truncate rel patch (v8).
> Here are other comments for the latest patch:
> 
> + block = visibilitymap_truncate_prepare(rel, 0); if
> + (BlockNumberIsValid(block)) fork = VISIBILITYMAP_FORKNUM;
> +
> + smgrtruncate(rel->rd_smgr, &fork, 1, &block);
> 
> If visibilitymap_truncate_prepare() returns InvalidBlockNumber,
> smgrtruncate() should not be called.
> 
> + FreeSpaceMapVacuumRange(rel, first_removed_nblocks,
> + InvalidBlockNumber);

Thank you again for the review!

Regards,
Kirk Jamison

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [HACKERS] [PATCH] pageinspect function to decode infomasks
Next
From: Tatsuro Yamada
Date:
Subject: Re: [HACKERS] CLUSTER command progress monitor