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.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com