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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, 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-13 15:59:40
Message-ID: 20210813155940.kp7mdg3ynl2hodqk@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Hi,

(dropping -committers to avoid moderation stalls due xposting to multiple lists -
I find that more annoying than helpful)

On 2021-08-13 14:38:37 +0530, Amit Kapila wrote:
> > What I'm wondering is why it is a good idea to have a SharedFileSet specific
> > cleanup mechanism. One that only operates on process lifetime level, rather
> > than something more granular. I get that the of the files here needs to be
> > longer than a transaction, but that can easily be addressed by having a longer
> > lived resource owner.
> >
> > Process lifetime may work well for the current worker.c, but even there it
> > doesn't seem optimal. One e.g. could easily imagine that we'd want to handle
> > connection errors or configuration changes without restarting the worker, in
> > which case process lifetime obviously isn't a good idea anymore.
> >
>
> I don't deny that we can't make this at a more granular level. IIRC,
> at that time, we tried to follow AtProcExit_Files which cleans up temp
> files at proc exit and we needed something similar for temporary files
> used via SharedFileSet.

The comparison to AtProcExit_Files isn't convincing to me - normally temp
files are cleaned up long before that via resowner cleanup.

To me the reuse of SharedFileSet for worker.c as executed seems like a bad
design. As things stand there's little code shared between dsm/non-dsm shared
file sets. The cleanup semantics are entirely different. Several functions
don't work if used on the "wrong kind" of set (e.g. SharedFileSetAttach()).

> 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?

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2021-08-13 18:00:23 pgsql: Add RISC-V spinlock support in s_lock.h.
Previous Message Peter Eisentraut 2021-08-13 15:38:14 pgsql: pg_amcheck: Message style and structuring improvements

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-08-13 16:03:28 Re: [PATCH] Native spinlock support on RISC-V
Previous Message Bruce Momjian 2021-08-13 15:47:49 Re: Default to TIMESTAMP WITH TIME ZONE?