Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb - Mailing list pgsql-hackers

From Justin Pryzby
Subject Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb
Date
Msg-id 20210419115504.GD7256@telsasoft.com
Whole thread Raw
In response to Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb  (Amul Sul <sulamul@gmail.com>)
List pgsql-hackers
On Mon, Apr 19, 2021 at 04:27:25PM +0530, Amul Sul wrote:
> On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> >
> > At Mon, 19 Apr 2021 12:56:18 +0530, Amul Sul <sulamul@gmail.com> wrote in
> > > On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier <michael@paquier.xyz> wrote:
> > > >
> > > > On Fri, Apr 09, 2021 at 06:45:45PM -0400, Alvaro Herrera wrote:
> > > > > We forgot this patch earlier in the commitfest.  Do people think we
> > > > > should still get it in on this cycle?  I'm +1 on that, since it's a
> > > > > safety feature poised to prevent more bugs than it's likely to
> > > > > introduce.
> > > >
> > > > No objections from here to do that now even after feature freeze.  I
> > > > also wonder, while looking at that, why you don't just remove the last
> > > > call within src/backend/catalog/heap.c.  This way, nobody is tempted
> > > > to use RelationOpenSmgr() anymore, and it could just be removed from
> > > > rel.h.
> > >
> > > Agree, did the same in the attached version, thanks.
> >
> > +       smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, BLOOM_METAPAGE_BLKNO,
> >                           (char *) metapage, true);
> > -       log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
> > +       log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
> >
> > At the log_newpage, index is guaranteed to have rd_smgr. So I prefer
> > to leave the line alone..  I don't mind other sccessive calls if any
> > since what I don't like is the notation there.
> >
> 
> Perhaps, isn't that bad. It is good to follow the practice of using
> RelationGetSmgr() for rd_smgr access, IMHO.
> 
> > > P.S. commitfest entry https://commitfest.postgresql.org/33/3084/
> >
> > Isn't this a kind of open item?
> >
> 
> Sorry, I didn't get you. Do I need to move this to some other bucket?

It's not a new feature, and shouldn't wait for July's CF since it's targetting
v14.

The original crash was fixed by Tom by commit 9d523119f.

So it's not exactly an "open item" for v14, but there's probably no better
place for it, so you could add it if you think it's at risk of being forgotten
(again).

https://wiki.postgresql.org/wiki/PostgreSQL_14_Open_Items

-- 
Justin



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Table refer leak in logical replication
Next
From: Amit Kapila
Date:
Subject: Re: Table refer leak in logical replication