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

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?

Regards,
Amul