Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps  (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1rNl9M-00FOFN-BV
	for pgsql-hackers@arkaria.postgresql.org; Thu, 11 Jan 2024 02:50:33 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.94.2)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1rNl9L-008iMx-FV
	for pgsql-hackers@arkaria.postgresql.org; Thu, 11 Jan 2024 02:50:31 +0000
Received: from makus.postgresql.org ([2001:4800:3e1:1::229])
	by malur.postgresql.org with esmtps  (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <a.lepikhov@postgrespro.ru>)
	id 1rNl9L-008iMJ-1I
	for pgsql-hackers@lists.postgresql.org; Thu, 11 Jan 2024 02:50:31 +0000
Received: from mail.postgrespro.ru ([93.174.131.139])
	by makus.postgresql.org with esmtps  (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <a.lepikhov@postgrespro.ru>)
	id 1rNl9H-000wJY-Jp
	for pgsql-hackers@postgresql.org; Thu, 11 Jan 2024 02:50:29 +0000
Received: from [192.168.1.58] (node-1cyd.pool-101-51.dynamic.totinternet.net [101.51.247.213])
	(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
	(Client did not present a certificate)
	(Authenticated sender: a.lepikhov@postgrespro.ru)
	by mail.postgrespro.ru (Postfix/587) with ESMTPSA id D048CE20FD8;
	Thu, 11 Jan 2024 05:50:22 +0300 (MSK)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru;
	s=mx2023; t=1704941424;
	bh=w0JyZUGGHVJMbdtMWCbQyuquEbkO6a7SpKB4IxrkEts=;
	h=Message-ID:Date:User-Agent:Subject:To:Cc:References:From:
	 In-Reply-To:From;
	b=BqonYMTTwRRqsHWEi4cetlBCEMgOn7nAXRxyqw/ejvgc3oTTv6zd4K4KnnBuw/rLd
	 wzb8ETJrXYBoSmC4y3EWLqkFi3z/LU+EQ2tHcuf1Tp/rUhOy2JbkTLe/0iOZqVREP2
	 ShqoGW1jUGaupOeOYN4MupkaMguD0EBRPEWzOMEOME2Kv3XL/zNudJr3F+5t2fys2T
	 ITUB8J0m0ntM+bQncnvjeO+Er+ciJ0tKqi3HFCOPICSOfSf/GDR8Xx5aeda94eq0Ya
	 Aei0lj3cA7T5KSGdUrOvU4jcmEidzKJ4Kw9etknvRJV6QBaTeR7EulqFJYZ+PgpU5j
	 jXOglxlbyfO+A==
Message-ID: <a3f7230e-0e73-49f4-8c3a-bb6e01472f57@postgrespro.ru>
Date: Thu, 11 Jan 2024 09:50:19 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: introduce dynamic shared memory registry
Content-Language: en-US
To: Nathan Bossart <nathandbossart@gmail.com>,
 Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org
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>
 <20240108171627.GA2611898@nathanxps13>
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Organization: Postgres Professional
In-Reply-To: <20240108171627.GA2611898@nathanxps13>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: <https://www.postgresql.org/message-id/a3f7230e-0e73-49f4-8c3a-bb6e01472f57%40postgrespro.ru>
Precedence: bulk

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