On Mon, Jan 22, 2024 at 05:00:48PM +0530, Bharath Rupireddy wrote:
> On Mon, Jan 22, 2024 at 3:43 AM Nathan Bossart
> wrote:
>> Oops. I've attached an attempt at fixing this. I took the opportunity to
>> clean up the surrounding code a bit.
>
> The code looks cleaner and readable with the
On Mon, Jan 22, 2024 at 3:43 AM Nathan Bossart wrote:
>
> Oops. I've attached an attempt at fixing this. I took the opportunity to
> clean up the surrounding code a bit.
The code looks cleaner and readable with the patch. All the call sites
are taking care of dsm_attach returning NULL value.
On Sun, Jan 21, 2024 at 04:13:20PM -0600, Nathan Bossart wrote:
> Oops. I've attached an attempt at fixing this. I took the opportunity to
> clean up the surrounding code a bit.
Thanks for the patch. Your proposed attempt looks correct to me with
an ERROR when no segments are found..
--
On Sun, Jan 21, 2024 at 11:21:46AM -0500, Tom Lane wrote:
> Coverity complained about this:
>
> *** CID 1586660: Null pointer dereferences (NULL_RETURNS)
> /srv/coverity/git/pgsql-git/postgresql/src/backend/storage/ipc/dsm_registry.c:
> 185 in GetNamedDSMSegment()
> 179 }
> 180
Nathan Bossart writes:
> Committed. Thanks everyone for reviewing!
Coverity complained about this:
*** CID 1586660: Null pointer dereferences (NULL_RETURNS)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/ipc/dsm_registry.c:
185 in GetNamedDSMSegment()
179 }
180
On Wed, Jan 17, 2024 at 08:00:00AM +0530, Bharath Rupireddy wrote:
> The v8 patches look good to me.
Committed. Thanks everyone for reviewing!
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, Jan 16, 2024 at 9:37 PM Nathan Bossart wrote:
>
> The autoprewarm change (0003) does use this variable. I considered making
> it optional (i.e., you could pass in NULL if you didn't want it), but I
> didn't feel like the extra code in GetNamedDSMSegment() to allow this was
> worth it so
On Tue, Jan 16, 2024 at 10:28:29AM +0530, Bharath Rupireddy wrote:
> I think it's better for GetNamedDSMSegment() to error out on empty
> 'name' and size 0. This makes the user-facing function
> GetNamedDSMSegment more concrete.
Agreed, thanks for the suggestion.
> +void *
>
On Sun, Jan 14, 2024 at 3:11 AM Nathan Bossart wrote:
>
> Here is a new version of the patch set with these changes.
Thanks. Here are some comments on v7-0002.
1.
+GetNamedDSMSegment(const char *name, size_t size,
+ void (*init_callback) (void *ptr), bool *found)
+{
+
+
On Fri, Jan 12, 2024 at 01:45:55PM -0600, Nathan Bossart wrote:
> On Fri, Jan 12, 2024 at 11:13:46PM +0530, Abhijit Menon-Sen wrote:
>> At 2024-01-12 11:21:52 -0600, nathandboss...@gmail.com wrote:
>>> + Each backend sould obtain a pointer to the reserved shared memory by
>>
>> sould →
On Fri, Jan 12, 2024 at 11:13:46PM +0530, Abhijit Menon-Sen wrote:
> At 2024-01-12 11:21:52 -0600, nathandboss...@gmail.com wrote:
>> + Each backend sould obtain a pointer to the reserved shared memory by
>
> sould → should
D'oh. Thanks.
>> + Add-ins can reserve LWLocks on server
At 2024-01-12 11:21:52 -0600, nathandboss...@gmail.com wrote:
>
> From: Nathan Bossart
> Date: Thu, 11 Jan 2024 21:55:25 -0600
> Subject: [PATCH v6 1/3] reorganize shared memory and lwlocks documentation
>
> ---
> doc/src/sgml/xfunc.sgml | 182 +---
> 1 file
Here's a new version of the patch set in which I've attempted to address
the feedback in this thread. Note that 0001 is being tracked in a separate
thread [0], but it is basically a prerequisite for adding the documentation
for this feature, so that's why I've also added it here.
[0]
On Thu, Jan 11, 2024 at 10:42 AM Michael Paquier wrote:
>
> >> 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
On Mon, Jan 08, 2024 at 11:16:27AM -0600, 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 Shared Memory and
>> LWLocks section in
On Thu, Jan 11, 2024 at 09:50:19AM +0700, Andrei Lepikhov wrote:
> On 9/1/2024 00:16, Nathan Bossart wrote:
>> On Mon, Jan 08, 2024 at 10:53:17AM +0530, Bharath Rupireddy wrote:
>> > 2. FWIW, I'd like to call this whole feature "Support for named DSM
>> > segments in Postgres". Do you see anything
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 Shared Memory and
LWLocks section in xfunc.sgml? This will at least tell
On Mon, Jan 8, 2024 at 10:48 PM Nathan Bossart
wrote:
> On Mon, Jan 08, 2024 at 11:13:42AM +0530, Amul Sul wrote:
> > +void *
> > +dsm_registry_init_or_attach(const char *key, size_t size,
> >
> > I think the name could be simple as dsm_registry_init() like we use
> > elsewhere e.g.
On Mon, Jan 08, 2024 at 11:13:42AM +0530, Amul Sul wrote:
> +void *
> +dsm_registry_init_or_attach(const char *key, size_t size,
>
> I think the name could be simple as dsm_registry_init() like we use
> elsewhere e.g. ShmemInitHash() which doesn't say attach explicitly.
That seems reasonable to
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 Shared Memory and
> LWLocks section in xfunc.sgml? This will at least tell there's
> another way for external modules
On Mon, Jan 8, 2024 at 10:53 AM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> On Sat, Jan 6, 2024 at 10:05 PM Nathan Bossart
> 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
On Sat, Jan 6, 2024 at 10:05 PM Nathan Bossart 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
On Sat, Jan 06, 2024 at 07:34:15PM +0530, Bharath Rupireddy wrote:
> 1. Update all the copyright to the new year. A run of
> src/tools/copyright.pl on the source tree will take care of it at some
> point, but still it's good if we can update while we are here.
Done.
> 2. Typo: missing "an"
On Wed, Jan 3, 2024 at 4:19 AM Nathan Bossart wrote:
>
> Here's a new version of the patch set with Bharath's feedback addressed.
Thanks. The v4 patches look good to me except for a few minor
comments. I've marked it as RfC in CF.
1. Update all the copyright to the new year. A run of
Here's a new version of the patch set with Bharath's feedback addressed.
On Tue, Jan 02, 2024 at 11:31:14AM -0500, Robert Haas wrote:
> On Tue, Jan 2, 2024 at 11:21 AM Nathan Bossart
> wrote:
>> > Are we expecting, for instance, a 128-bit UUID being used as a key and
>> > hence limiting it to a
On Tue, Jan 2, 2024 at 11:21 AM Nathan Bossart wrote:
> > Are we expecting, for instance, a 128-bit UUID being used as a key and
> > hence limiting it to a higher value 256 instead of just NAMEDATALEN?
> > My thoughts were around saving a few bytes of shared memory space that
> > can get higher
On Fri, Dec 29, 2023 at 08:53:54PM +0530, Bharath Rupireddy wrote:
> With the use of dsm registry for pg_prewarm, do we need this
> test_dsm_registry module at all? Because 0002 patch pretty much
> demonstrates how to use the DSM registry. With this comment and my
> earlier comment on
On Wed, Dec 20, 2023 at 9:33 PM Nathan Bossart wrote:
>
> On Wed, Dec 20, 2023 at 03:28:38PM +0530, Bharath Rupireddy wrote:
> > Isn't the worker_spi best place to show the use of the DSM registry
> > instead of a separate test extension? Note the custom wait event
> > feature that added its
On Wed, Dec 27, 2023 at 01:53:27PM -0600, Nathan Bossart wrote:
> Here is a new version of the patch. In addition to various small changes,
> I've rewritten the test suite to use an integer and a lock, added a
> dsm_registry_find() function, and adjusted autoprewarm to use the registry.
Here's a
Here is a new version of the patch. In addition to various small changes,
I've rewritten the test suite to use an integer and a lock, added a
dsm_registry_find() function, and adjusted autoprewarm to use the registry.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From
On 20/12/2023 17:33, Nathan Bossart wrote:
On Wed, Dec 20, 2023 at 11:02:58AM +0200, Andrei Lepikhov wrote:
In that case, maybe change the test case to make it closer to real-life
usage - with locks and concurrent access (See attachment)?
I'm not following why we should make this test case
On Thu, Dec 21, 2023 at 12:03:18AM +0800, Zhang Mingli wrote:
> I see most xxxShmemInit functions have the logic to handle IsUnderPostmaster
> env.
> Do we need to consider it in DSMRegistryShmemInit() too? For example, add
> some assertions.
> Others LGTM.
Good point. I _think_ the registry
Hi, all
I see most xxxShmemInit functions have the logic to handle IsUnderPostmaster
env.
Do we need to consider it in DSMRegistryShmemInit() too? For example, add some
assertions.
Others LGTM.
Zhang Mingli
www.hashdata.xyz
On Dec 5, 2023 at 11:47 +0800, Nathan Bossart , wrote:
> Every once
On Wed, Dec 20, 2023 at 03:28:38PM +0530, Bharath Rupireddy wrote:
> Isn't the worker_spi best place to show the use of the DSM registry
> instead of a separate test extension? Note the custom wait event
> feature that added its usage code to worker_spi. Since worker_spi
> demonstrates typical
On Wed, Dec 20, 2023 at 11:02:58AM +0200, Andrei Lepikhov wrote:
> In that case, maybe change the test case to make it closer to real-life
> usage - with locks and concurrent access (See attachment)?
I'm not following why we should make this test case more complicated. It
is only intended to
On Tue, Dec 5, 2023 at 9:17 AM Nathan Bossart wrote:
>
> Every once in a while, I find myself wanting to use shared memory in a
> loadable module without requiring it to be loaded at server start via
> shared_preload_libraries. The DSM API offers a nice way to create and
> manage dynamic shared
On 20/12/2023 07:04, Michael Paquier wrote:
On Tue, Dec 19, 2023 at 10:14:44AM -0600, Nathan Bossart wrote:
On Tue, Dec 19, 2023 at 10:49:23AM -0500, Robert Haas wrote:
On Mon, Dec 18, 2023 at 3:32 AM Andrei Lepikhov
wrote:
2. I think a separate file for this feature looks too expensive.
On Tue, Dec 19, 2023 at 10:14:44AM -0600, Nathan Bossart wrote:
> On Tue, Dec 19, 2023 at 10:49:23AM -0500, Robert Haas wrote:
>> On Mon, Dec 18, 2023 at 3:32 AM Andrei Lepikhov
>> wrote:
>>> 2. I think a separate file for this feature looks too expensive.
>>> According to the gist of that code,
On Tue, Dec 19, 2023 at 10:19:11AM -0600, Nathan Bossart wrote:
> On Fri, Dec 08, 2023 at 04:36:52PM +0900, Michael Paquier wrote:
>> Yes, tracking that in a more central way can have many usages, so your
>> patch sounds like a good idea. Note that we have one case in core
>> that be improved and
On Fri, Dec 08, 2023 at 04:36:52PM +0900, Michael Paquier wrote:
> Yes, tracking that in a more central way can have many usages, so your
> patch sounds like a good idea. Note that we have one case in core
> that be improved and make use of what you have here: autoprewarm.c.
I'll add a patch for
On Tue, Dec 19, 2023 at 10:49:23AM -0500, Robert Haas wrote:
> On Mon, Dec 18, 2023 at 3:32 AM Andrei Lepikhov
> wrote:
>> 2. I think a separate file for this feature looks too expensive.
>> According to the gist of that code, it is a part of the DSA module.
>
> -1. I think this is a totally
On Mon, Dec 18, 2023 at 12:05:28PM +0300, Nikita Malakhov wrote:
> Just a suggestion - maybe it is worth adding a function for detaching the
> segment,
> for cases when we unload and/or re-load the extension?
Hm. We don't presently have a good way to unload a library, but you can
certainly DROP
On Mon, Dec 18, 2023 at 03:32:08PM +0700, Andrei Lepikhov wrote:
> 3. The dsm_registry_init_or_attach routine allocates a DSM segment. Why not
> create dsa_area for a requestor and return it?
My assumption is that most modules just need a fixed-size segment, and if
they really needed a DSA
On Mon, Dec 18, 2023 at 3:32 AM Andrei Lepikhov
wrote:
> 2. I think a separate file for this feature looks too expensive.
> According to the gist of that code, it is a part of the DSA module.
-1. I think this is a totally different thing than DSA. More files
aren't nearly as expensive as the
Hi!
This patch looks like a good solution for a pain in the ass, I'm too for
this patch to be committed.
Have looked through the code and agree with Andrei, the code looks good.
Just a suggestion - maybe it is worth adding a function for detaching the
segment,
for cases when we unload and/or
On 18/12/2023 13:39, Andrei Lepikhov wrote:
On 5/12/2023 10:46, Nathan Bossart wrote:
I don't presently have any concrete plans to use this for anything, but I
thought it might be useful for extensions for caching, etc. and wanted to
see whether there was any interest in the feature.
I am
On 5/12/2023 10:46, Nathan Bossart wrote:
I don't presently have any concrete plans to use this for anything, but I
thought it might be useful for extensions for caching, etc. and wanted to
see whether there was any interest in the feature.
I am delighted that you commenced this thread.
On Mon, Dec 04, 2023 at 09:46:47PM -0600, Nathan Bossart wrote:
> The attached 0001 introduces a "DSM registry" to solve this problem. The
> API provides an easy way to allocate/initialize a segment or to attach to
> an existing one. The registry itself is just a dshash table that stores
> the
On Tue, Dec 5, 2023 at 10:35 AM Joe Conway wrote:
> Notwithstanding any dragons there may be, and not having actually looked
> at the the patches, I love the concept! +
Seems fine to me too. I haven't looked at the patches or searched for
dragons either, though.
--
Robert Haas
EDB:
On 12/4/23 22:46, Nathan Bossart wrote:
Every once in a while, I find myself wanting to use shared memory in a
loadable module without requiring it to be loaded at server start via
shared_preload_libraries. The DSM API offers a nice way to create and
manage dynamic shared memory segments, so
Every once in a while, I find myself wanting to use shared memory in a
loadable module without requiring it to be loaded at server start via
shared_preload_libraries. The DSM API offers a nice way to create and
manage dynamic shared memory segments, so creating a segment after server
start is
51 matches
Mail list logo
