Re: Better shared data structure management and resizable shared data structures - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: Better shared data structure management and resizable shared data structures
Date
Msg-id 249b4f57-bbb6-4030-a299-26013b9f7d44@iki.fi
Whole thread Raw
In response to Re: Better shared data structure management and resizable shared data structures  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
On 27/03/2026 02:51, Heikki Linnakangas wrote:
> On 25/03/2026 20:37, Robert Haas wrote:
>> On Sat, Mar 21, 2026 at 8:14 PM Heikki Linnakangas <hlinnaka@iki.fi> 
>> wrote:>> Shmem callbacks
>>> ---------------
>>>
>>> I separated the request/init/fn callbacks from the structs. There's now
>>> a concept of "shmem callbacks", which you register in _PG_init(). For
>>> example:
>>>
>>> static void pgss_shmem_request(void *arg);
>>> static void pgss_shmem_init(void *arg);
>>>
>>> static const ShmemCallbacks pgss_shmem_callbacks = {
>>>       .request_fn = pgss_shmem_request,
>>>       .init_fn = pgss_shmem_init,
>>>       .attach_fn = NULL, /* no special attach actions needed */
>>> };
>>
>> What's the advantage of coupling the functions together this way, vs.
>> just registering each callback individually?
> 
> One reason is to support allocations after postmaster startup. The 
> RegisterShmemCallbacks() call ties together all the resources requested 
> by the request_fn callback, with the the init_fn or attach_fn callbacks 
> that will later initialize/attach them. The init_fn/attach_fn callbacks 
> are called only after *all* the resources requested by the request_fn 
> callback have been initialized, and it holds a lock while doing all that.
> 
> If the callbacks were registered separately, shmem.c wouldn't know when 
> to call the init_fn/attach_fn. There's no problem during postmaster or 
> backend startup, because we run all init_fn or attach_fn callbacks in 
> the whole system, after requesting all the resources, but after startup, 
> you must only call the callbacks related to the newly-requested resources.
> 
> Aside from that after-startup allocation issue, though, IMO the 
> ShmemCallbacks struct makes it more clear that the callbacks are meant 
> to work together on the same resources.
> 
> One way to think of this is that all the resources requested by the 
> request_fn callback are implicitly part of the same "subsystem", and 
> need to be initialized/attached to together. We discussed that before, 
> and I still wonder if we should make that concept of a subsystem more 
> explicit. If we just renamed ShmemCallbacks to ShmemSubsystem, and give 
> each subsystem a name, it'd look like this:
> 
> static void pgss_shmem_request(void *arg);
> static void pgss_shmem_init(void *arg);
> 
> static const ShmemSubsystem pgss_shmem_subsystem = {
>      .name = "pg_stat_statements"
>      .request_fn = pgss_shmem_request,
>      .init_fn = pgss_shmem_init,
>      .attach_fn = NULL, /* no special attach actions needed */
> };
> 
> static void
> pgss_shmem_request(void *arg)
> {
>      ShmemRequestStruct(&pgssSharedStateDesc, &(ShmemRequestStructOpts) {
>           /*
>            * name is optional in this design, subsystem's name is used if
>            * not given
>            */
>          .name = "pg_stat_statements",
>          .size = sizeof(pgssSharedState),
>          .ptr = (void **) &pgss,
>      });
> }
> 
> static void
> pgss_shmem_init(void *arg)
> {
>      /* initialize contents of pgss */
>      ...
> }
> 
> void
> _PG_init(void)
> {
>      RegisterShmemSubsystem(&pgss_shmem_subsystem);
> }
> 
> 
> 
> Thinking how this might work without such a struct, registering the 
> callbacks separately, here's an alternative design:
> 
> static void pgss_shmem_request(void *arg);
> static void pgss_shmem_init(void *arg);
> 
> static void
> pgss_shmem_request(void *arg)
> {
>      ShmemRequestStruct(&pgssSharedStateDesc, &(ShmemRequestStructOpts) {
>          .name = "pg_stat_statements",
>          .size = sizeof(pgssSharedState),
>          .ptr = (void **) &pgss,
>      });
> 
>      ShmemRegisterInitCallback(&pgss_shmem_init);
>      /* no attach callback needed, but for illustration: */
>      ShmemRegisterInitCallback(&pgss_shmem_attach);
> }
> 
> static void
> pgss_shmem_init(void *arg)
> {
>      /* initialize contents of pgss */
>      ...
> }
> 
> void
> _PG_init(void)
> {
>      ShmemRegisterRequestCallback(&pgss_shmem_request);
> }
> 
> In this design, the ShmemRegisterRequestCallback() call still ties 
> together all the related resources. All the resources requested in the 
> request-callback are initialized together, and the fact that the init/ 
> attach callbacks are registered within the request callback associates 
> them with the resources. This feels a little Rube Goldbergian, with one 
> callback registering more callbacks, but would also work.

Thinking about this some more, we could also just pass the callback 
functions directly as arguments to the ShmemRegisterCallback() function, 
without the ShmemCallbacks struct. If they're all passed in one call, 
that still ties them together. The shmem.c implementation would probably 
still need the ShmemCallbacks struct, but that would be a detail 
internal to shmem.c. It would look like this:

static void pgss_shmem_request(void *arg);
static void pgss_shmem_init(void *arg);

static void
pgss_shmem_request(void *arg)
{
      ShmemRequestStruct(
          .name = "pg_stat_statements",
          .size = sizeof(pgssSharedState),
          .ptr = (void **) &pgss,
      );
}


static void
pgss_shmem_init(void *arg)
{
      /* initialize contents of pgss */
      ...
}


void
_PG_init(void)
{
      RegisterShmemSubsystem(pgss_shmem_request,
                             pgss_shmem_init,
                             NULL, /* no attach fn needed */
                             0, /* flags */);
}

This is pretty much the same as what's in the latest patch version, but 
a little less boilerplate as you don't need the ShmemCallbacks struct. 
The struct would be useful if we had needs to add lots of optional 
options in the future, but I don't think we have such needs.

- Heikki




pgsql-hackers by date:

Previous
From: Etsuro Fujita
Date:
Subject: Re: Options to control remote transactions’ access/deferrable modes in postgres_fdw
Next
From: Etsuro Fujita
Date:
Subject: Re: Import Statistics in postgres_fdw before resorting to sampling.