Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Amul Sul <sulamul(at)gmail(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, Neha Sharma <neha(dot)sharma(at)enterprisedb(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb
Date: 2021-04-19 11:55:04
Message-ID: 20210419115504.GD7256@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(dot)ntt(at)gmail(dot)com> wrote:
> >
> > At Mon, 19 Apr 2021 12:56:18 +0530, Amul Sul <sulamul(at)gmail(dot)com> wrote in
> > > On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier <michael(at)paquier(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-04-19 11:57:02 Re: Table refer leak in logical replication
Previous Message Michael Paquier 2021-04-19 11:50:21 Re: Table refer leak in logical replication