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

Hi,

On 9/5/22 10:32 AM, Kyotaro Horiguchi wrote:
> At Mon, 5 Sep 2022 08:52:44 +0200, "Drouvot, Bertrand"<bdrouvot(at)amazon(dot)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