From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
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>, pgsql-committers <pgsql-committers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o |
Date: | 2021-08-13 09:08:37 |
Message-ID: | CAA4eK1Khx7nvqSS-inKB0WGbYxX1sy6EkzzxR-QmnVmFqnmmXw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On Thu, Aug 12, 2021 at 6:18 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2021-08-12 15:06:23 +0530, Amit Kapila wrote:
> > On Thu, Aug 12, 2021 at 1:52 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > I'm not so sure. Why does sharedfileset have its own proc exit hook in the
> > > first place? ISTM that this should be dealt with using resowners, rathers than
> > > a sharedfileset specific mechanism?
> > >
>
> > The underlying temporary files need to be closed at xact end but need
> > to survive across transactions.
>
> Why do they need to be closed at xact end? To avoid wasting memory due to too
> many buffered files?
>
Yes.
>
> > These are registered with the resource owner via
> > PathNameOpenTemporaryFile/PathNameCreateTemporaryFile and then closed
> > at xact end. So, we need a way to remove the files used by the process
> > (apply worker in this particular case) before process exit and used
> > this proc_exit hook (possibly on the lines of AtProcExit_Files).
>
> 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. 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.
>
> I think SharedFileSetInit() needs a comment explaining that it needs to be
> called in a process-lifetime memory context if used without dsm
> segments.
>
We already have a comment about proc_exit clean up of files but will
extend that a bit about memory context.
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2021-08-13 09:09:35 | Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o |
Previous Message | Michael Meskes | 2021-08-13 08:46:31 | pgsql: Fix connection handling for DEALLOCATE and DESCRIBE statements |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2021-08-13 09:09:35 | Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o |
Previous Message | Peter Eisentraut | 2021-08-13 08:52:50 | Re: Shared memory size computation oversight? |