Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o - Mailing list pgsql-committers

From Amit Kapila
Subject Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Date
Msg-id CAA4eK1JHUoE997NdUXbS9ikHk1oOtPupnNpGGnN_P3pn+jb6LQ@mail.gmail.com
Whole thread Raw
In response to Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o  (Dilip Kumar <dilipbalaut@gmail.com>)
Responses Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o  (Masahiko Sawada <sawada.mshk@gmail.com>)
List pgsql-committers
On Thu, Aug 12, 2021 at 11:38 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Aug 12, 2021 at 7:39 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Aug 11, 2021 at 10:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, Aug 10, 2021 at 4:37 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > The apply worker registers SharedFileSetDeleteOnProcExit() when
> > > > creating a file set to serialize the changes. When it raises an error
> > > > due to conflict during applying the change, the callback eventually
> > > > reports the temp file statistics but pgstat already shut down,
> > > > resulting in this assertion failure.
> > > >
> > >
> > > I think we can try to fix this by registering to clean up these files
> > > via before_shmem_exit() as done by Andres in commit 675c945394.
> > > Similar to that commit, we can change the function name
> > > SharedFileSetDeleteOnProcExit to SharedFileSetDeleteOnShmExit and
> > > register it via before_shmem_exit() instead of on_proc_exit(). Can you
> > > try that and see if it fixes the issue for you unless you have better
> > > ideas to try out?
> >
> > 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 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?

-- 
With Regards,
Amit Kapila.



pgsql-committers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Next
From: Masahiko Sawada
Date:
Subject: Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o