Re: Patch to address creation of PgStat* contexts with null parent context

At Thu, 04 Aug 2022 13:12:32 -0400, Reid Thompson <reid(dot)thompson(at)crunchydata(dot)com> wrote in
> On Fri, 2022-07-29 at 11:53 +0900, Kyotaro Horiguchi wrote:
> >
> > That makes the memorycontext-tree structure unstable because
> > CacheMemoryContext can be created on-the-fly.
> >
> > Honestly I don't like to call CreateCacheMemoryContext in the two
> > functions on-the-fly.  Since every process that calls
> > pgstat_initialize() necessarily calls pgstat_setup_memcxt() at latest
> > at process termination, I think we can create at least
> > CacheMemoryContext in pgstat_initialize().
>
> Attached is a patch creating CacheMemoryContext() in pgstat_initialize()
> rather than the two previous patch locations.
>
> > Or couldn't we create the
> > all three contexts in the function, instead of calling
> > pgstat_setup_memcxt() on-the fly?
>
> You note that that pgstat_setup_memcxt() is called at latest at process
> termination -- was the intent to hold off on requesting memory for these
> two contexts until it was needed?

I think it a bit different. Previously that memory (but for a bit
different use, precisely) was required only when stats data is read so
almost all server processes didn't need it. Now, every server process
that uses pgstats requires the two memory if it is going to write
stats. Even if that didn't happen until process termination, that
memory eventually required to flush possibly remaining data. That
final write might be avoidable but I'm not sure it's worth the
trouble. As the result, calling pgstat_initialize() is effectively
the declaration that the process requires the memory.

Thus I thought that we may let pgstat_initialize() promptly allocate
the memory.

Does it make sense?

About the patch, I had something like the attached in my mind.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center