Hi,

In [1] I looked at pgstat memory usage after sort of a complaint by Nathan.
The conversion of "PgStat Shared Ref" to slab seems like an improvement we
obviously should make [2].

In the email I also showed how much CacheMemoryContext using a lot of memory
after doing a database wide VACUUM (with ~100k tables), not very surprising.


I didn't immediately look at the next few entries:
┌────────────────────────┬─────────────┬───────────────┬────────────┬─────────────┬────────────┐
│          name          │ total_bytes │ total_nblocks │ free_bytes │ 
free_chunks │ used_bytes │
├────────────────────────┼─────────────┼───────────────┼────────────┼─────────────┼────────────┤
│ CacheMemoryContext     │   168820784 │            31 │    2363064 │          
33 │  166457720 │
│ PgStat Pending         │    27983872 │          3419 │   27846912 │      
112699 │     136960 │
│ smgr relation table    │    16777216 │            12 │    2797592 │          
50 │   13979624 │
│ MdSmgr                 │     8388608 │            11 │    3475256 │           
1 │    4913352 │
│ PgStat Shared Ref      │     5767344 │            88 │      19712 │         
352 │    5747632 │
│ PgStat Shared Ref Hash │     4195376 │             2 │        712 │           
0 │    4194664 │
│ Relcache by OID        │     4194304 │            10 │      57448 │          
24 │    4136856 │
│ TopMemoryContext       │      173264 │             7 │      32968 │          
79 │     140296 │
└────────────────────────┴─────────────┴───────────────┴────────────┴─────────────┴────────────┘

When I did I was quite surprised to see "PgStat Pending" using so much, as it
should all be freed after the stats have been submitted, a few seconds later
at most.

After a moment of worry about having introduced a leak, and adding the
necessary columns to see more information, it's clear that all the memory was
actually freed. It's just that we don't ever free aset blocks, even if all the
constituent memory has been freed. So the overall context size doesn't shrink.

That seems decidedly not great.

I wonder how bad it would be to teach aset to recognize this situation.

But I think for this use case we actually have a more fitting memory context
type for this workload, i.e. GenerationContext.  With that the size after the
same vacuum is

│ PgStat Pending         │      134144 │             3 │     133848 │           
0 │        296 │

Seems like a fairly obvious change.


My local experimental changes attached.


Greetings,

Andres Freund

[1] 
https://postgr.es/m/fcvnawwq32mamvf66q5i3sk73xudxz5corqlgqljtocepspjps%40ypvl6yjzy5xk
[2] It's a big enough saving that I'm kinda wondering about whether we should
try to sneak it into 19.
diff --git i/src/backend/utils/activity/pgstat.c w/src/backend/utils/activity/pgstat.c
index eb8ccbaa628..fbfa0e8fc3c 100644
--- i/src/backend/utils/activity/pgstat.c
+++ w/src/backend/utils/activity/pgstat.c
@@ -1300,10 +1300,19 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat
 
 	if (unlikely(!pgStatPendingContext))
 	{
+#if 0
 		pgStatPendingContext =
 			AllocSetContextCreate(TopMemoryContext,
 								  "PgStat Pending",
 								  ALLOCSET_SMALL_SIZES);
+#else
+		pgStatPendingContext =
+			GenerationContextCreate(TopMemoryContext,
+									"PgStat Pending",
+									1024,
+									1024,
+									128 * 1024);
+#endif
 	}
 
 	entry_ref = pgstat_get_entry_ref(kind, dboid, objid,
diff --git i/src/backend/utils/activity/pgstat_shmem.c w/src/backend/utils/activity/pgstat_shmem.c
index b8f354c818a..df279665d62 100644
--- i/src/backend/utils/activity/pgstat_shmem.c
+++ w/src/backend/utils/activity/pgstat_shmem.c
@@ -1179,10 +1179,20 @@ static void
 pgstat_setup_memcxt(void)
 {
 	if (unlikely(!pgStatSharedRefContext))
+	{
+#if 0
 		pgStatSharedRefContext =
 			AllocSetContextCreate(TopMemoryContext,
 								  "PgStat Shared Ref",
 								  ALLOCSET_SMALL_SIZES);
+#else
+		pgStatSharedRefContext =
+			SlabContextCreate(TopMemoryContext,
+							  "PgStat Shared Ref",
+							  16*1024,
+							  sizeof(PgStat_EntryRef));
+#endif
+	}
 	if (unlikely(!pgStatEntryRefHashContext))
 		pgStatEntryRefHashContext =
 			AllocSetContextCreate(TopMemoryContext,

Reply via email to