Horton

Thread

Commits

  1. Fix shmem allocation of fixed-sized custom stats kind

  2. Fix shared memory size of template code for custom fixed-sized pgstats

  1. Shmem allocated wrong for custom cumulative stats

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-04-06T00:16:47Z

    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

  2. Re: Shmem allocated wrong for custom cumulative stats

    Michael Paquier <michael@paquier.xyz> — 2026-04-06T00:55:14Z

    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

  3. Re: Shmem allocated wrong for custom cumulative stats

    Michael Paquier <michael@paquier.xyz> — 2026-04-06T02:36:21Z

    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

  4. Re: Shmem allocated wrong for custom cumulative stats

    Michael Paquier <michael@paquier.xyz> — 2026-04-06T03:14:06Z

    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

  5. Re: Shmem allocated wrong for custom cumulative stats

    Michael Paquier <michael@paquier.xyz> — 2026-04-07T03:04:35Z

    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