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

On Thu, Aug 12, 2021 at 1:52 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> On 2021-08-12 11:46:09 +0530, Amit Kapila wrote:
> > On Thu, Aug 12, 2021 at 11:38 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > On Thu, Aug 12, 2021 at 7:39 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > > It seems to me that moving the shared fileset cleanup to
> > > > before_shmem_exit() is the right approach to fix this problem. The
> > > > issue is fixed by the attached patch.
> > >
> > > +1, the fix makes sense to me.
>
> 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. 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).

> That said, I think it's fine to go for the ordering change in the short term.
>
>
> > I have also tested and fix works for me. The fix works because
> > pgstat_initialize() is called before we register clean up in
> > SharedFileSetInit(). I am not sure if we need an Assert to ensure that
> > and if so how we can do that? Any suggestions?
>
> I don't think we need to assert that - we'd see failures soon enough if
> that rule were violated...
>

Fair enough.

--
With Regards,
Amit Kapila.