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

Hi,

At Mon, 5 Sep 2022 08:52:44 +0200, "Drouvot, Bertrand" <bdrouvot@amazon.com> wrote in

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.

Of course it works. The difference from what I last proposed is
whether we postpone creating the memory contexts until the first call
to pgstat_get_entry_ref(). 

Right.

 The rationale of creating them at
pgstat_attach_shmem is that anyway once pgstat_attach_shmem is called,
the process fainally creates the contexts at the end of the process,

Right.

IIUC the downside is to allocate the new contexts even for processes that don't need them (as mentioned by Andres upthread).

and (I think) it's simpler that we don't do if() check at every
pgstat_get_entry_ref() call.

I wonder how much of a concern the if() checks are, given they are all 3 legitimately using unlikely().

Looks like that both approaches have their pros and cons. I'm tempted to vote +1 on "just changing" the parent context to TopMemoryContext and not changing the allocations locations.

Regards,

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