Обсуждение: Proposal: Add a callback data parameter to GetNamedDSMSegment

Поиск
Список
Период
Сортировка

Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Zsolt Parragi
Дата:
Hello hackers,

While developing an extension and trying to write some generic code
around DSM areas, I noticed a limitation: although GetNamedDSMSegment
accepts a callback function, there is no way to pass additional
arguments to that callback.

For example, the documentation for creating LWLocks after startup [1]
suggests creating locks in this callback. That works fine as long as
the callback only needs to create a hardcoded lock. But if the lock
name is a parameter to the function calling GetNamedDSMSegment, and
not fixed, I do not see a clean way to pass it through to the callback
(short of relying on global variables, for example).

As a proper solution for this, and possibly for other similar issues,
I propose adding a generic callback argument to GetNamedDSMSegment
that can be forwarded to the callback function.

What do you think about this?

[1] https://www.postgresql.org/docs/current/xfunc-c.html#XFUNC-ADDIN-LWLOCKS-AFTER-STARTUP

Вложения

Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Sami Imseih
Дата:
Hi,

Can you provide more details on the use-case?

> For example, the documentation for creating LWLocks after startup [1]
> suggests creating locks in this callback. That works fine as long as
> the callback only needs to create a hardcoded lock.

The callback is called on the first invocation of GetNamedDSMSegment for
a particular segment name. Subsequent calls just attach an existing segment.

> But if the lock name is a parameter to the function calling GetNamedDSMSegment, and
> not fixed, I do not see a clean way to pass it through to the callback

Keep in mind that the tranche name shows up in wait events, so you
will end up with different wait event names.

Also,

commit 38b602b capped the number of lwlock tranches to 256, so you
may hit this limit if you are creating many lwlocks.

#define MAX_NAMED_TRANCHES 256


--
Sami Imseih
Amazon Web services (AWS)



Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Nathan Bossart
Дата:
On Wed, Dec 03, 2025 at 12:47:46PM -0600, Sami Imseih wrote:
> Can you provide more details on the use-case?

I think the main use-case is creating multiple DSM segments in the registry
that use the same initialization callback.  I ran into this when I was
working on GetNamedDSA() and GetNamedDSHash().  In early versions of the
patch, the new functions used GetNamedDSMSegment() to allocate the space
for all the DSA/dshash information [0].  Since initializing those segments
required the user-provided name string, I ended up taking a lock after
calling GetNamedDSMSegment() and doing most of the initialization there.
My gut feeling is that this is an obscure enough use-case that this
workaround is probably sufficient, but I am interested to hear more...

[0] https://postgr.es/m/attachment/177621/v8-0001-simplify-creating-hash-table-in-dsm-registry.patch

-- 
nathan



Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Sami Imseih
Дата:
> My gut feeling is that this is an obscure enough use-case that this
> workaround is probably sufficient, but I am interested to hear more...

There are probably other good reasons to allow a generic argument
to the init callback. Besides the lwlock name, I can see someone
wanting to pass some other initialization info that may vary
depending on extension GUCs, etc.

--
Sami



Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Nathan Bossart
Дата:
On Wed, Dec 03, 2025 at 02:27:29PM -0600, Sami Imseih wrote:
> There are probably other good reasons to allow a generic argument
> to the init callback. Besides the lwlock name, I can see someone
> wanting to pass some other initialization info that may vary
> depending on extension GUCs, etc.

The value of a GUC could be obtained in the callback via the global
variable or GetConfigOption(), right?

-- 
nathan



Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Sami Imseih
Дата:
> On Wed, Dec 03, 2025 at 02:27:29PM -0600, Sami Imseih wrote:
> > There are probably other good reasons to allow a generic argument
> > to the init callback. Besides the lwlock name, I can see someone
> > wanting to pass some other initialization info that may vary
> > depending on extension GUCs, etc.
>
> The value of a GUC could be obtained in the callback via the global
> variable or GetConfigOption(), right?

Yes, that's true. It will be hard to find other good use-cases that
can't be solved with a global variable, but we can also say this is
a low-cost change, so why not just do it.

--
Sami



Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Nathan Bossart
Дата:
On Wed, Dec 03, 2025 at 02:59:16PM -0600, Sami Imseih wrote:
> Yes, that's true. It will be hard to find other good use-cases that
> can't be solved with a global variable, but we can also say this is
> a low-cost change, so why not just do it.

Well, for one, it requires all existing extensions that use
GetNamedDSMSegment() to be updated.  That might not be too terrible, but in
any case, I think we need a stronger reason than the simplicity of the
implementation to do something.

-- 
nathan



Re: Proposal: Add a callback data parameter to GetNamedDSMSegment

От
Zsolt Parragi
Дата:
> I ran into this when I was
> working on GetNamedDSA() and GetNamedDSHash()

Thanks for mentioning these, I didn't notice them when I rebased on
the master branch. One of my use cases was this, I implemented
something similar to GetNamedDSHash - it's a generic wrapper for
dshash in C++, and I ran into the same issue.

> Besides the lwlock name, I can see someone
> wanting to pass some other initialization info that may vary
> depending on extension GUCs, etc.

Yes, that was my thought too. GUCs/globals can be accessed directly,
but there's still the reusing the same function part. and also
calculated local variables.

My other use case is using GetNamedDSMSegment without DSHash, with a
flexible array, similar to:

struct Foo {
    LWLock lock;
    size_t size;
    Bar data[];
};

* To create a few of these, I have to provide a lock name to the
callback, that's the "reusing the same callback" part again
* And then there's the question of initialization. Either I leave it
to the caller after returning from GetNamedDSHash using the lock, or
somehow I have to tell the initialization callback the array size -
even if I can calculate the size based on a GUC, I wouldn't want to
trust that instead of the actual information from the caller.

> Well, for one, it requires all existing extensions that use
> GetNamedDSMSegment() to be updated.  That might not be too terrible, but in
> any case, I think we need a stronger reason than the simplicity of the
> implementation to do something.

I did some searching for usage before my initial email, and I only
found 3 extensions that use this method, which seemed like a small
number. Also, the LWLockInitialize change already requires ifdefs in
this function for PG19.

But maybe the second use case alone is too specific to justify this
change, and there are workarounds to it.