Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Date: 2021-08-18 04:17:40
Message-ID: CAFiTN-vmzxtRGSVdUj2TNYuh-Vzhe3ChH9oBSBHA0cG=NKghog@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Wed, Aug 18, 2021 at 9:30 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Aug 18, 2021 at 8:23 AM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Wed, Aug 18, 2021 9:17 AM houzj(dot)fnst(at)fujitsu(dot)com wrote:
> > > Hi,
> > >
> > > I took a quick look at the v2 patch and noticed a typo.
> > >
> > > + * backends and render it read-only. If missing_ok is true then it will return
> > > + * NULL if file doesn not exist otherwise error.
> > > */
> > > doesn not=> doesn't
> > >
> >
> > Here are some other comments:
> >
> > 1).
> > +BufFileOpenShared(SharedFileSet *fileset, const char *name, int mode,
> > + bool missing_ok)
> > {
> > BufFile *file;
> > char segment_name[MAXPGPATH];
> > ...
> > files = palloc(sizeof(File) * capacity);
> > ...
> > @@ -318,10 +320,15 @@ BufFileOpenShared(SharedFileSet *fileset, const char *name, int mode)
> > * name.
> > */
> > if (nfiles == 0)
> > + {
> > + if (missing_ok)
> > + return NULL;
> > +
> >
> > I think it might be better to pfree(files) when return NULL.
> >
> >
> > 2).
> > /* Delete the subxact file and release the memory, if it exist */
> > - if (ent->subxact_fileset)
> > - {
> > - subxact_filename(path, subid, xid);
> > - SharedFileSetDeleteAll(ent->subxact_fileset);
> > - pfree(ent->subxact_fileset);
> > - ent->subxact_fileset = NULL;
> > - }
> > -
> > - /* Remove the xid entry from the stream xid hash */
> > - hash_search(xidhash, (void *) &xid, HASH_REMOVE, NULL);
> > + subxact_filename(path, subid, xid);
> > + SharedFileSetDelete(xidfileset, path, true);
> >
> > Without the patch it doesn't throw an error if not exist,
> > But with the patch, it pass error_on_failure=true to SharedFileSetDelete().
> >
>
> Don't we need to use BufFileDeleteShared instead of
> SharedFileSetDelete as you have used to remove the changes file?

Yeah, it should be BufFileDeleteShared.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Amit Kapila 2021-08-18 05:54:01 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Previous Message Amit Kapila 2021-08-18 04:00:09 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2021-08-18 04:29:54 Re: Skipping logical replication transactions on subscriber side
Previous Message Amit Kapila 2021-08-18 04:00:09 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o