| From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| Cc: | Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, chaturvedipalak1911(at)gmail(dot)com |
| Subject: | Re: Better shared data structure management and resizable shared data structures |
| Date: | 2026-04-06 13:53:19 |
| Message-ID: | CAExHW5u5LOgB3Y4Ee3VWVEurYN2GwnkbVEcfrGCQLgcGgf_zKw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Apr 6, 2026 at 4:58 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 05/04/2026 23:06, Heikki Linnakangas wrote:
> > Here's patch version 12 [*]. I believe I've addressed all the feedback,
> > and I feel this is in pretty good shape now. There hasn't been any big
> > design changes lately.
> >
> > One notable change is that I replaced the separate {request|init|attach}
> > _fn_arg fields in ShmemCallbacks with a single 'opaque_arg' field, and
> > added a brief comment to it. You both commented on whether we need that
> > at all, and maybe you're right that we don't, but at least it's now just
> > one field rather than three. As before, callers can simply ignore it if
> > they don't need it.
>
> After another round of comment cleanups and such, committed. Thanks!
Thanks. Attached are rebased patches.
0001 is the same patch as submitted before to support resizable shared
memory structures.
0002 changes resizable_shmem_usage() in the resizable_shmem test
module to use /proc/self/smaps as suggested by Andres offline. Using
smaps the function estimates the actual memory mapped against the main
shared memory segment. Expecting this to fix failure reported on
https://cirrus-ci.com/task/5501660157444096.
0003 adds some more diagnostic about shared memory segments in the
server log to debug the failure in case it appears even with 0002
0004 addresses Matthias's comments in the discussion below
On Mon, Apr 6, 2026 at 12:35 AM Matthias van de Meent
<boekewurm+postgres(at)gmail(dot)com> wrote:
>
> On Sun, 5 Apr 2026 at 13:20, Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >
> > On Sun, Apr 5, 2026 at 2:36 PM Matthias van de Meent
> > <boekewurm+postgres(at)gmail(dot)com> wrote:
> > >
> > > On Sun, 5 Apr 2026, 07:59 Ashutosh Bapat, <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > > >
> > > > On Sun, Apr 5, 2026 at 11:18 AM Ashutosh Bapat
> > > > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > > > >
> > >
> > > I'm not opposed to HAVE_RESIZABLE_SHMEM, but is it universal enough on
> > > its platforms to make it part of the exposed ABI for Shmem? I think
> > > that we should expose the same functions and structs, and just have
> > > the shmem internals throw an error if the configuration used by the
> > > user implies the user wants to update shmem sizing when the system
> > > doesn't support it. That would avoid extensions having to recompile
> > > between have/have not systems that have an otherwise compatible ABI;
> > > especially when those extensions don't actually need the resizeable
> > > part of the shmem system.
> > >
> >
> > I don't think I understand this fully. An extension may want to
> > support a structure in both modes - fixed as well as resizable
> > depending upon whether the latter is supported. If the structure has
> > maximum_size always the extension code needs to set it to 0 when the
> > resizable shared structure is not supported and set to actual
> > maximum_size when the resizable structure is supported. Without a
> > macro or some flag they can not do that. The flag/macro then becomes
> > part ABI for shmem. Am I correct?
>
> That's not quite what I meant.
>
> With your patch, the size and field offsets in `struct
> ShmemStructOpts` changes depending only on HAVE_RESIZABLE_SHMEM, as
> does function's availability. This means that an extension that's
> built without HAVE_RESIZABLE_SHMEM (an otherwise identical system)
> can't correctly be loaded into a server that does have
> HAVE_RESIZABLE_SHMEM defined - or at least it'll misbehave when it
> tries to use the new shmem system without trying out resizeable areas.
>
> If instead the fields used for definining resizable shmem areas (and
> the relevant functions) are always defined, but with runtime checks to
> make sure that in !HAVE_RESIZEABLE_SHMEM nobody tries to use the
> resizing functionality, then that'd reduce the unchecked hidden
> incompatibility; assuming that no extension manually does memory
> management syscall operations on those shmem areas.
>
> > Since extension binaries need to be
> > built on different platforms anyway, that would automatically take
> > care of building with or without HAVE_RESIZABLE_SHMEM. I feel it makes
> > testing simpler since run time behaviour is fixed. Maybe I am missing
> > something. Maybe a code diff or some example platform might make it
> > more clear for me.
>
> I'm not entirely sure it would be automatic. Is it guaranteed that
> HAVE_RESIZABLE_SHMEM won't change over the lifetime of any
> distribution's platform? Because it's definitely not apparent to me
> that rebuilding the new server version against an upgraded platform
> (now possibly with HAVE_RESIZABLE_SHMEM) should also mean rebuilding
> the extensions that have been built against a previous minor version
> (without HAVE_RESIZABLE_SHMEM).
>
Please review changes in 0004 to see if they address your concerns.
The structures and functions are same irrespective of
HAVE_RESIZABLE_SHMEM. If HAVE_RESIZABLE_SHMEM is not defined, but
maximum_size > 0, then the request to add this structure will fail.
Thus on a server running with binary built with HAVE_RESIZABLE_SHMEM
undefined, maximum_size = 0 always. Many of the code blocks don't need
#ifdef HAVE_RESIZABLE_SHMEM blocks then. I think code this way is much
more readable. However binary will contain more instructions than
necessary - maybe the compiler can optimize those out.
> > > > For now, it
> > > > seems only for the sanity checks, but it could be seen as a useful
> > > > safety feature. A difference in maximum_size and minimum_size would
> > > > indicate that the structure is resizable.
> > >
> > > I think that's the right approach.
> >
> >
> > I also think that introducing minimum_size is useful. Let's hear from
> > Heikki before implementing it, in case he has a different opinion. I
> > am not sure about min_allocated_space though - what use do you see for
> > it. reserved_space is useful in pg_shmem_allocations() C function
> > itself and gives impact to the fully grown structure. What would
> > min_allocated_space give us? If at all it would be min_allocated_size
> > not space since reserved space will never change. But even that I am
> > not sure about.
>
> I'd say it's mostly interesting for people looking at or debugging
> shmem allocations. Which isn't a huge group of developers or DBAs, but
> if we're exposing data like this, and are going to allow resizing,
> then someone could see some benefits from this.
>
> E.g., it may be useful to have the information to see how low the
> currently running server can scale down its memory usage, so that the
> admin can see whether a reboot is required if they want to allow it to
> scale it down further (assuming there's a lower limit for allocations
> - some shmem structs may have a lower scaling limit defined at
> startup, while others may be able to scale linearly from 0 to 100)
I guess, most of the time the lower limit will be a hard lower limit
which could not be changed, so I guess the usecases are pretty narrow.
0005 adds minimum_size member alongside maximum_size as per the
discussion starting [1]. I like the end result since maximum_size is
managed consistently across fixed-size and resizable structures and
also the condition to check whether a structure is resizable or fixed
is more natural/logical.
0006 adds support to add mprotect appropriately on the used and unused
part of a resizable structure, again per discussion starting [1]. This
is still a bit rough, but review comments are welcome.
I have kept these two patches separate from the main patch so that I
can remove them if others feel they are not worth including in the
feature.
--
Best Wishes,
Ashutosh Bapat
| Attachment | Content-Type | Size |
|---|---|---|
| v20260406-0001-resizable-shared-memory-structures.patch | text/x-patch | 66.6 KB |
| v20260406-0005-Add-minimum_size-specification-for-resizab.patch | text/x-patch | 19.3 KB |
| v20260406-0004-Avoid-creating-ABI-incompatibility-because.patch | text/x-patch | 14.4 KB |
| v20260406-0003-Add-more-diagnostics-about-shared-memory-s.patch | text/x-patch | 4.3 KB |
| v20260406-0002-Use-smaps-instead-of-status-in-resizable_s.patch | text/x-patch | 6.1 KB |
| v20260406-0006-Add-support-to-protect-unused-resizable_sh.patch | text/x-patch | 11.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2026-04-06 13:56:23 | Re: pg_plan_advice |
| Previous Message | Bruce Momjian | 2026-04-06 13:40:01 | Re: PG 19 release notes and authors |