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

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: 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-17 06:36:37
Message-ID: CAA4eK1LXHYn49DOMonwO0PQxbhzMTri_eDzwc6dxm6jwueFZkw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Tue, Aug 17, 2021 at 10:54 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Aug 16, 2021 at 8:18 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Fri, Aug 13, 2021 at 9:29 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > > > I think we can extend this API but I guess it is better to then do it
> > > > for dsm-based as well so that these get tracked via resowner.
> > >
> > > DSM segments are resowner managed already, so it's not obvious that that'd buy
> > > us much? Although I guess there could be a few edge cases that'd look cleaner,
> > > because we could reliably trigger cleanup in the leader instead of relying on
> > > dsm detach hooks + refcounts to manage when a set is physically deleted?
> > >
> >
> > In an off-list discussion with Thomas and Amit, we tried to discuss
> > how to clean up the shared files set in the current use case. Thomas
> > suggested that instead of using individual shared fileset for storing
> > the data for each XID why don't we just create a single shared fileset
> > for complete worker lifetime and when the worker is exiting we can
> > just remove that shared fileset. And for storing XID data, we can
> > just create the files under the same shared fileset and delete those
> > files when we longer need them. I like this idea and it looks much
> > cleaner, after this, we can get rid of the special cleanup mechanism
> > using 'filesetlist'. I have attached a patch for the same.
> >
>
> It seems to me that this idea would obviate any need for resource
> owners as we will have only one fileset now. I have a few initial
> comments on the patch:
>

One more comment:
@@ -2976,39 +2952,17 @@ subxact_info_write(Oid subid, TransactionId xid)
..
+ /* Try to open the subxact file, if it doesn't exist then create it */
+ fd = BufFileOpenShared(xidfileset, path, O_RDWR, true);
+ if (fd == NULL)
+ fd = BufFileCreateShared(xidfileset, path);
..

Instead of trying to create the file here based on whether it exists
or not, can't we create it in subxact_info_add where we are first time
allocating memory for subxacts? If that works then in the above code,
the file should always exist.

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Dilip Kumar 2021-08-17 07:37:45 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Previous Message Amit Kapila 2021-08-17 05:24:30 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o

Browse pgsql-hackers by date

  From Date Subject
Next Message 李杰 (慎追) 2021-08-17 07:06:09 回复:A problem in ExecModifyTable
Previous Message Michael Paquier 2021-08-17 06:34:28 Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE