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

At Sat, 6 Aug 2022 19:19:39 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> Hi,
>
> On 2022-08-05 17:22:38 +0900, Kyotaro Horiguchi wrote:
> > 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.
>
> I don't think every process will end up calling pgstat_setup_memcxt() -
> e.g. walsender, bgwriter, checkpointer probably don't? What do we gain by
> creating the contexts eagerly?

Yes. they acutally does, in shmem_shutdown hook function, during
at-termination stats write. I didn't consider to make that not
happen, to save 2kB of memory on such small number of processes.

> > Thus I thought that we may let pgstat_initialize() promptly allocate
> > the memory.
>
> That makes some sense - but pgstat_attach_shmem() seems like a very strange
> place for the call to CreateCacheMemoryContext().

Sure. (I hesitantly added #include for catcache.h..)

> I wonder if we shouldn't just use TopMemoryContext as the parent for most of
> these contexts instead. CacheMemoryContext isn't actually a particularly good
> fit anymore.

It looks better than creating CacheMemoryContext. Now
pgstat_initialize() creates the memory contexts for pgstats use under
TopMemoryContext.

And we don't hastle to avoid maybe-empty at-process-termination
writes..

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center