On Sat, Jan 6, 2024 at 10:05 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > > I kept this the same, as I didn't see any need to tie the key size to > NAMEDATALEN.
Thanks. A fresh look at the v5 patches left me with the following thoughts:
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?
2. FWIW, I'd like to call this whole feature "Support for named DSM segments in Postgres". Do you see anything wrong with this?
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?
4. With the understanding in comment #3, can pg_stat_statements and test_slru.c get rid of its shmem_request_hook and shmem_startup_hook and use dsm_registry_init_or_attach? It's not that this patch need to remove them now, but just asking if there's something in there that makes this new feature unusable.
+1, since doing for pg_prewarm, better to do for these modules as well.