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

Hi,

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



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?

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().


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.

Could using TopMemoryContext like in the attach be an option? (aka changing CacheMemoryContext by TopMemoryContext in the 3 places of interest): that would ensure the 3 pgStat* contexts to have a non NULL parent context.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com