[prev in list] [next in list] [prev in thread] [next in thread] 

List:       postgresql-hackers
Subject:    Patch to address creation of PgStat* contexts with null parent context
From:       Reid Thompson <reid.thompson () crunchydata ! com>
Date:       2022-06-30 19:54:50
Message-ID: 81ce164a81a03da9078ae4530caafd5593525df5.camel () crunchydata ! com
[Download RAW message or body]

[Attachment #2 (text/plain)]

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


[Attachment #3 (unknown)]

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 0d9d09c492..bf53eba660 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -102,6 +102,7 @@
 #include "storage/lwlock.h"
 #include "storage/pg_shmem.h"
 #include "storage/shmem.h"
+#include "utils/catcache.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
@@ -1059,6 +1060,9 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, Oid objoid, bool *created
 
 	if (unlikely(!pgStatPendingContext))
 	{
+		if (unlikely(!CacheMemoryContext))
+			CreateCacheMemoryContext();
+
 		pgStatPendingContext =
 			AllocSetContextCreate(CacheMemoryContext,
 								  "PgStat Pending",
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 89060ef29a..6462510f11 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -14,6 +14,7 @@
 
 #include "pgstat.h"
 #include "storage/shmem.h"
+#include "utils/catcache.h"
 #include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
 
@@ -974,6 +975,9 @@ pgstat_reset_entries_of_kind(PgStat_Kind kind, TimestampTz ts)
 static void
 pgstat_setup_memcxt(void)
 {
+	if (unlikely(!CacheMemoryContext))
+		CreateCacheMemoryContext();
+
 	if (unlikely(!pgStatSharedRefContext))
 		pgStatSharedRefContext =
 			AllocSetContextCreate(CacheMemoryContext,


[prev in list] [next in list] [prev in thread] [next in thread]