Shmem allocated wrong for custom cumulative stats
There's only one call of ShmemAlloc() left in the tree outside shmem.c,
in StatsShmemInit(), and that call doesn't look right:
The StatsShmemSize() function takes those allocations into account when
it calculates the size for the "Shared Memory Stats" shmem area, but
ShmemAlloc() doesn't use that reservation, it uses the general-purpose
unreserved shmem that then shows up as "<anonymous>" in
pg_shmem_allocations. The space reserved with ShmemRequestStruct() /
ShmemInitStruct() goes unused.
We should use the memory that we've reserved, per the attached patch.
One consequence of this fix though is that the allocations are now only
MAXALIGNed, while ShmemAlloc() uses CACHELINEALIGN(). Not sure which we
want.
I noticed this while working on the new shmem allocation functions, but
it's a pre-existing bug in stable branches too.
- Heikki
Attachments:
0001-Use-the-allocated-space-properly-for-custom-stats-ki.patchtext/x-patch; charset=UTF-8; name=0001-Use-the-allocated-space-properly-for-custom-stats-ki.patchDownload+3-2
On Mon, Apr 06, 2026 at 03:16:47AM +0300, Heikki Linnakangas wrote:
We should use the memory that we've reserved, per the attached patch. One
consequence of this fix though is that the allocations are now only
MAXALIGNed, while ShmemAlloc() uses CACHELINEALIGN(). Not sure which we
want.
Indeed, it's not right. Thanks for the report. I am pretty sure that
I intended each chunk to be MAXALIGN()-d for each custom stats kind
registered, allocated in a non-anonymous way, without cache alignment.
I noticed this while working on the new shmem allocation functions, but it's
a pre-existing bug in stable branches too.
Right, down to v18 where this has been introduced. That's my bug, so
I'd be OK to take care of it myself, if you are OK with that of
course.
Actually, shouldn't StatsShmemSize() use an add_size() for each
shared_size? Noted while passing through the code, extra error from
the same commit.
--
Michael
Attachments:
v2-0001-Use-the-allocated-space-properly-for-custom-stats.patchtext/plain; charset=us-asciiDownload+4-4
On Mon, Apr 06, 2026 at 09:55:14AM +0900, Michael Paquier wrote:
Right, down to v18 where this has been introduced. That's my bug, so
I'd be OK to take care of it myself, if you are OK with that of
course.
Note: something is wrong with -m32, for both patches. Digging into
that..
--
Michael
On Mon, Apr 06, 2026 at 11:36:21AM +0900, Michael Paquier wrote:
On Mon, Apr 06, 2026 at 09:55:14AM +0900, Michael Paquier wrote:
Right, down to v18 where this has been introduced. That's my bug, so
I'd be OK to take care of it myself, if you are OK with that of
course.Note: something is wrong with -m32, for both patches. Digging into
that..
And I have been puzzled for a few minutes here, trying to figure out
if this was something in v18 or something with the new shmem routines.
It is nothing of the kind: test_custom_stats has been underestimating
its shared_size, using PgStat_StatCustomFixedEntry instead of
PgStatShared_CustomFixedEntry. Interesting copy-pasto, HEAD-only,
second bug.
The attached is working correctly. The v18 flavor is slightly
simpler.
--
Michael
On Mon, Apr 06, 2026 at 12:14:06PM +0900, Michael Paquier wrote:
The attached is working correctly. The v18 flavor is slightly
simpler.
I have looked at it again today, and noticed that the test modules of
both HEAD and REL_18_STABLE were incorrect in the values they set for
shared_size for the fixed-sized cases. In both cases we were
finishing with an underestimation. I could only see a direct impact
on HEAD for the 32-bit builds, though, but the problem was the same.
The allocation has been fixed in 17132f55c5a2, with the modules
handled by 98979578055f.
--
Michael