Re: Patch to address creation of PgStat* contexts with null parent context - Mailing list pgsql-hackers

From Zhang Mingli
Subject Re: Patch to address creation of PgStat* contexts with null parent context
Date
Msg-id fd7cc8ae-89d4-44b9-ba1d-7371268d011f@Spark
Whole thread Raw
In response to Patch to address creation of PgStat* contexts with null parent context  (Reid Thompson <reid.thompson@crunchydata.com>)
Responses Re: Patch to address creation of PgStat* contexts with null parent context
List 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

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
Next
From: vignesh C
Date:
Subject: Re: Handle infinite recursion in logical replication setup