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

Hi,

On Jul 28, 2022, 21:30 +0800, Reid Thompson <reid(dot)thompson(at)crunchydata(dot)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