Date: Mon, 8 Jan 2024 11:16:27 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: introduce dynamic shared memory registry
Message-ID: <20240108171627.GA2611898@nathanxps13>
References: <20231205034647.GA2705267@nathanxps13>
 <CALj2ACW6vLFFNa-+jeAOy9p7SvQQmcud2A7H2zUMQyL35Go=Yg@mail.gmail.com>
 <20231220160324.GB833819@nathanxps13>
 <CALj2ACVz2s6LKpfrBHWvPX-7Z2MzgyOQtjsqbgHWTk+ic5JpiA@mail.gmail.com>
 <20240102162054.GB955987@nathanxps13>
 <CA+TgmoZRber4XLQC6PaRneh5OECakguePcFD4YUg8qJ+jB+u=g@mail.gmail.com>
 <20240102224907.GA1246933@nathanxps13>
 <CALj2ACWc3Xb6v06T2jkJvEoA9W4aQ+0fY-8w78bY_Dq1ux6GDg@mail.gmail.com>
 <20240106163516.GB2435448@nathanxps13>
 <CALj2ACW4AcGcbwFzFr4RZ62PtcOj=Xg73Xmy_YDQya3g=xVDhQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CALj2ACW4AcGcbwFzFr4RZ62PtcOj=Xg73Xmy_YDQya3g=xVDhQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20240108171627.GA2611898%40nathanxps13>
Precedence: bulk

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?

Good call.  I definitely think this stuff ought to be documented.  After a
quick read, I also wonder if it'd be worth spending some time refining that
section.

> 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."

> 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.

> 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.

It might be possible, but IIUC you'd still need a way to know whether the
library was preloaded, i.e., all the other necessary hooks were in place.
It's convenient to just be able to check whether the shared memory was set
up for this purpose.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com