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

Hi,

On 8/7/22 4:19 AM, Andres Freund wrote:
> 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