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

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: zmlpostgres@gmail.com
Cc: pgsql-hackers@lists.postgresql.org, reid.thompson@crunchydata.com
Date: 2022-07-29T02:53:56Z
Lists: pgsql-hackers
At Thu, 28 Jul 2022 22:03:13 +0800, Zhang Mingli <zmlpostgres@gmail.com> wrote in 
> Hi,
> 
> On Jul 28, 2022, 21:30 +0800, Reid Thompson <reid.thompson@crunchydata.com>, wrote:
> > Attached is a patch to address this.

Good Catch!

> Codes seem good, my question is:
> 
> Do auto vacuum processes need CacheMemoryContext?

pgstat_report_vacuum requires it. Startup process doesn't seem to use
pgstats while recovery proceeding but requires the context only at
termination...

> Is it designed not to  create CacheMemoryContext in such processes?
> 
> If so, we’d better use TopMemoryContext in such processes.

That makes the memorycontext-tree structure unstable because
CacheMemoryContext can be created on-the-fly.

Honestly I don't like to call CreateCacheMemoryContext in the two
functions on-the-fly.  Since every process that calls
pgstat_initialize() necessarily calls pgstat_setup_memcxt() at latest
at process termination, I think we can create at least
CacheMemoryContext in pgstat_initialize(). Or couldn't we create the
all three contexts in the function, instead of calling
pgstat_setup_memcxt() on-the fly?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Commits

  1. pgstat: Create memory contexts below TopMemoryContext