Re: Patch to address creation of PgStat* contexts with null parent context
Zhang Mingli <zmlpostgres@gmail.com>
From: Zhang Mingli <zmlpostgres@gmail.com>
To: pgsql-hackers@lists.postgresql.org, reid.thompson@crunchydata.com
Date: 2022-07-28T14:03:13Z
Lists: pgsql-hackers
Hi,
On Jul 28, 2022, 21:30 +0800, Reid Thompson <reid.thompson@crunchydata.com>, wrote:
> Hi,
>
> There are instances where pgstat_setup_memcxt() and
> pgstat_prep_pending_entry() are invoked before the CacheMemoryContext
> has been created. This results in PgStat* contexts being created
> without a parent context. Most easily reproduced/seen in autovacuum
> worker via pgstat_setup_memcxt().
>
> Attached is a patch to address this.
>
> To see the issue one can add a line similar to this to the top of
> MemoryContextCreate() in mcxt.c
> fprintf(stderr, "pid: %d ctxname: %s parent is %s CacheMemoryContext is %s\n", MyProcPid, name, parent == NULL ? "NULL" : "not NULL", CacheMemoryContext == NULL ? "NULL" : "Not NULL")
> and startup postgres and grep for the above after autovacuum workers
> have been invoked
>
> ...snip...
> pid: 1427756 ctxname: PgStat Pending parent is NULL CacheMemoryContext is NULL
> pid: 1427756 ctxname: PgStat Shared Ref parent is NULL CacheMemoryContext is NULL
> ...snip...
>
> or
>
> startup postgres, attach gdb to postgres following child, break at
> pgstat_setup_memcxt and wait for autovacuum worker to start...
>
> ...snip...
> ─── Source ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> 384 AllocSetContextCreateInternal(MemoryContext parent,
> 385 const char *name,
> 386 Size minContextSize,
> 387 Size initBlockSize,
> 388 Size maxBlockSize)
> 389 {
> 390 int freeListIndex;
> 391 Size firstBlockSize;
> 392 AllocSet set;
> 393 AllocBlock block;
> ─── Stack ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> [0] from 0x000055b7e4088b40 in AllocSetContextCreateInternal+0 at /home/rthompso/src/git/postgres/src/backend/utils/mmgr/aset.c:389
> [1] from 0x000055b7e3f41c88 in pgstat_setup_memcxt+2544 at /home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_shmem.c:979
> [2] from 0x000055b7e3f41c88 in pgstat_get_entry_ref+2648 at /home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_shmem.c:410
> [3] from 0x000055b7e3f420ea in pgstat_get_entry_ref_locked+26 at /home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_shmem.c:598
> [4] from 0x000055b7e3f3e2c4 in pgstat_report_autovac+36 at /home/rthompso/src/git/postgres/src/backend/utils/activity/pgstat_database.c:68
> [5] from 0x000055b7e3e7f267 in AutoVacWorkerMain+807 at /home/rthompso/src/git/postgres/src/backend/postmaster/autovacuum.c:1694
> [6] from 0x000055b7e3e7f435 in StartAutoVacWorker+133 at /home/rthompso/src/git/postgres/src/backend/postmaster/autovacuum.c:1493
> [7] from 0x000055b7e3e87367 in StartAutovacuumWorker+557 at /home/rthompso/src/git/postgres/src/backend/postmaster/postmaster.c:5539
> [8] from 0x000055b7e3e87367 in sigusr1_handler+935 at /home/rthompso/src/git/postgres/src/backend/postmaster/postmaster.c:5244
> [9] from 0x00007fb02bca7420 in __restore_rt
> [+]
> ─── Threads ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> [1] id 1174088 name postgres from 0x000055b7e4088b40 in AllocSetContextCreateInternal+0 at /home/rthompso/src/git/postgres/src/backend/utils/mmgr/aset.c:389
> ─── Variables ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> arg parent = 0x0, name = 0x55b7e416f179 "PgStat Shared Ref": 80 'P', minContextSize = 0, initBlockSize = 1024, maxBlockSize = 8192
> loc firstBlockSize = <optimized out>, set = <optimized out>, block = <optimized out>, __FUNCTION__ = "AllocSetContextCreateInternal", __func__ = "AllocSetContextCreateInternal"
> ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> > > > print CacheMemoryContext == NULL
> $4 = 1
> > > > print parent
> $5 = (MemoryContext) 0x0
>
> Thanks,
> Reid
>
>
Codes seem good, my question is:
Do auto vacuum processes need CacheMemoryContext?
Is it designed not to create CacheMemoryContext in such processes?
If so, we’d better use TopMemoryContext in such processes.
Regards,
Zhang Mingli
Commits
-
pgstat: Create memory contexts below TopMemoryContext
- fb503793ef50 15.0 landed
- 9d3ebba729eb 16.0 landed