Re: introduce dynamic shared memory registry

On 9/1/2024 00:16, Nathan Bossart wrote:
> On Mon, Jan 08, 2024 at 10:53:17AM +0530, Bharath Rupireddy wrote:
>> 1. I think we need to add some notes about this new way of getting
>> shared memory for external modules in the <title>Shared Memory and
>> LWLocks</title> section in xfunc.sgml? This will at least tell there's
>> another way for external modules to get shared memory, not just with
>> the shmem_request_hook and shmem_startup_hook. What do you think?
+1. Maybe even more - in the section related to extensions, this 
approach to using shared data can be mentioned, too.

>> 2. FWIW, I'd like to call this whole feature "Support for named DSM
>> segments in Postgres". Do you see anything wrong with this?
> 
> Why do you feel it should be renamed?  I don't see anything wrong with it,
> but I also don't see any particular advantage with that name compared to
> "dynamic shared memory registry."
It is not a big issue, I suppose. But for me personally (as not a native 
English speaker), the label "Named DSM segments" seems more 
straightforward to understand.
> 
>> 3. IIUC, this feature eventually makes both shmem_request_hook and
>> shmem_startup_hook pointless, no? Or put another way, what's the
>> significance of shmem request and startup hooks in lieu of this new
>> feature? I think it's quite possible to get rid of the shmem request
>> and startup hooks (of course, not now but at some point in future to
>> not break the external modules), because all the external modules can
>> allocate and initialize the same shared memory via
>> dsm_registry_init_or_attach and its init_callback. All the external
>> modules will then need to call dsm_registry_init_or_attach in their
>> _PG_init callbacks and/or in their bg worker's main functions in case
>> the modules intend to start up bg workers. Am I right?
> 
> Well, modules might need to do a number of other things (e.g., adding
> hooks) that can presently only be done when preloaded, in which case I
> doubt there's much benefit from switching to the DSM registry.  I don't
> really intend for it to replace the existing request/startup hooks, but
> you're probably right that most, if not all, could use the registry
> instead.  IMHO this is well beyond the scope of this thread, though.
+1, it may be a many reasons to use these hooks.

 >> 3. Use NAMEDATALEN instead of 64?
 >> +    char        key[64];
 > I kept this the same, as I didn't see any need to tie the key size to
 > NAMEDATALEN.
IMO, we should avoid magic numbers whenever possible. Current logic 
according to which first users of this feature will be extensions 
naturally bonds this size to the size of the 'name' type.

And one more point. I think the commit already deserves a more detailed 
commit message.

-- 
regards,
Andrei Lepikhov
Postgres Professional