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
-
pgstat: Create memory contexts below TopMemoryContext
- fb503793ef50 15.0 landed
- 9d3ebba729eb 16.0 landed