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 delighted that you commenced this thread.
Designing extensions, every time I feel pain introducing one shared
value or some global stat, the extension must be required to be loadable
on startup only. It reduces the flexibility of even very lightweight
extensions, which look harmful to use in a cloud.
After looking into the code, I have some comments:
1. The code looks good; I didn't find possible mishaps. Some proposed
changes are in the attachment.
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.
3. The dsm_registry_init_or_attach routine allocates a DSM segment. Why
not create dsa_area for a requestor and return it?
--
regards,
Andrei Lepikhov
Postgres Professional
diff --git a/src/backend/storage/ipc/dsm_registry.c
b/src/backend/storage/ipc/dsm_registry.c
index ea80f45716..0343ce987f 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -45,8 +45,8 @@ static const dshash_parameters dsh_params = {
LWTRANCHE_DSM_REGISTRY_HASH
};
-static dsa_area *dsm_registry_dsa;
-static dshash_table *dsm_registry_table;
+static dsa_area *dsm_registry_dsa = NULL;
+static dshash_table *dsm_registry_table = NULL;
static void init_dsm_registry(void);
@@ -83,13 +83,20 @@ init_dsm_registry(void)
{
/* Quick exit if we already did this. */
if (dsm_registry_table)
+ {
+ Assert(dsm_registry_dsa != NULL);
return;
+ }
+
+ Assert(dsm_registry_dsa == NULL);
/* Otherwise, use a lock to ensure only one process creates the table.
*/
LWLockAcquire(DSMRegistryLock, LW_EXCLUSIVE);
if (DSMRegistryCtx->dshh == DSHASH_HANDLE_INVALID)
{
+ Assert(DSMRegistryCtx->dsah == DSHASH_HANDLE_INVALID);
+
/* Initialize dynamic shared hash table for registry. */
dsm_registry_dsa = dsa_create(LWTRANCHE_DSM_REGISTRY_DSA);
dsa_pin(dsm_registry_dsa);
@@ -102,6 +109,8 @@ init_dsm_registry(void)
}
else
{
+ Assert(DSMRegistryCtx->dsah != DSHASH_HANDLE_INVALID);
+
/* Attach to existing dynamic shared hash table. */
dsm_registry_dsa = dsa_attach(DSMRegistryCtx->dsah);
dsa_pin_mapping(dsm_registry_dsa);
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index 665d471418..e0e7b3b765 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -209,7 +209,7 @@ typedef enum BuiltinTrancheIds
LWTRANCHE_LAUNCHER_HASH,
LWTRANCHE_DSM_REGISTRY_DSA,
LWTRANCHE_DSM_REGISTRY_HASH,
- LWTRANCHE_FIRST_USER_DEFINED
+ LWTRANCHE_FIRST_USER_DEFINED,
} BuiltinTrancheIds;
/*
