pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nathan Bossart <nathandbossart@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Rahila Syed <rahilasyed90@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Improve LWLock tranche name visibility across backends
Date: Tue, 11 Nov 2025 13:28:08 -0600
Message-ID: <aROOSLQwamg339K-@nathan> (raw )
In-Reply-To: <CAA5RZ0vj154vZkhT7y6z_hA3wnzOTpOjampbryk7FvC_SNNS=w@mail.gmail.com >
References: <CAA5RZ0t6s2Z38oRq883Ocq7J4xjn1FJF17Q4YR6++vrw5KYCtQ@mail.gmail.com >
<aLn4n-5UFzKHFdR5@nathan >
<dd36d384-55df-4fc2-825c-5bc56c950fa9@gmail.com >
<CAA5RZ0u8+sC_FBwoaUeLhTi5bmjba9Yk3N-TbdsTm7c1no0zcg@mail.gmail.com >
<CAA5RZ0v4kwF06o4a+Xybd++ipC7TYkn879Wz_2BZK412+ULXgg@mail.gmail.com >
<CAA5RZ0va93+wk3_6UYi52=b=xqbKtXvaM-Cj5moTir7nM-LA=Q@mail.gmail.com >
<aRISKd2MEeIqRcOh@nathan >
<aRIXtvkYn1mmyNG5@nathan >
<aRIpWji1L87hhBVe@nathan >
<CAA5RZ0vj154vZkhT7y6z_hA3wnzOTpOjampbryk7FvC_SNNS=w@mail.gmail.com >
On Mon, Nov 10, 2025 at 05:17:47PM -0600, Sami Imseih wrote:
> Maybe we can improve the error message by showing
> the "name", like this, In case, there are nested calls.
>
> ```
> else if (!entry->initialized)
> {
> ereport(ERROR,
> (errmsg("requested DSM segment \"%s\" failed initialization", name)));
> }
> ```
Done.
--
nathan
From 0b12db92e17dd315cc08b5a918196b685e3f6f61 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 10 Nov 2025 11:43:51 -0600
Subject: [PATCH v3 1/2] DSM registry: ERROR if entry was not initialized.
---
src/backend/storage/ipc/dsm_registry.c | 34 +++++++++++++++++++++++---
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index a926b9c3f32..7eba8a8cffb 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -93,6 +93,7 @@ typedef struct DSMRegistryEntry
{
char name[NAMEDATALEN];
DSMREntryType type;
+ bool initialized;
union
{
NamedDSMState dsm;
@@ -216,6 +217,7 @@ GetNamedDSMSegment(const char *name, size_t size,
dsm_segment *seg;
entry->type = DSMR_ENTRY_TYPE_DSM;
+ entry->initialized = false;
/* Initialize the segment. */
seg = dsm_create(size, 0);
@@ -228,13 +230,21 @@ GetNamedDSMSegment(const char *name, size_t size,
if (init_callback)
(*init_callback) (ret);
+
+ entry->initialized = true;
}
else if (entry->type != DSMR_ENTRY_TYPE_DSM)
ereport(ERROR,
- (errmsg("requested DSM segment does not match type of existing entry")));
+ (errmsg("requested DSM segment \"%s\" does not match type of existing entry",
+ name)));
+ else if (!entry->initialized)
+ ereport(ERROR,
+ (errmsg("requested DSM segment \"%s\" failed initialization",
+ name)));
else if (entry->dsm.size != size)
ereport(ERROR,
- (errmsg("requested DSM segment size does not match size of existing segment")));
+ (errmsg("requested DSM segment \"%s\" does not match size of existing entry",
+ name)));
else
{
NamedDSMState *state = &entry->dsm;
@@ -297,6 +307,7 @@ GetNamedDSA(const char *name, bool *found)
NamedDSAState *state = &entry->dsa;
entry->type = DSMR_ENTRY_TYPE_DSA;
+ entry->initialized = false;
/* Initialize the LWLock tranche for the DSA. */
state->tranche = LWLockNewTrancheId(name);
@@ -308,10 +319,17 @@ GetNamedDSA(const char *name, bool *found)
/* Store handle for other backends to use. */
state->handle = dsa_get_handle(ret);
+
+ entry->initialized = true;
}
else if (entry->type != DSMR_ENTRY_TYPE_DSA)
ereport(ERROR,
- (errmsg("requested DSA does not match type of existing entry")));
+ (errmsg("requested DSA \"%s\" does not match type of existing entry",
+ name)));
+ else if (!entry->initialized)
+ ereport(ERROR,
+ (errmsg("requested DSA \"%s\" failed initialization",
+ name)));
else
{
NamedDSAState *state = &entry->dsa;
@@ -372,6 +390,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
dsa_area *dsa;
entry->type = DSMR_ENTRY_TYPE_DSH;
+ entry->initialized = false;
/* Initialize the LWLock tranche for the hash table. */
dsh_state->tranche = LWLockNewTrancheId(name);
@@ -389,10 +408,17 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
/* Store handles for other backends to use. */
dsh_state->dsa_handle = dsa_get_handle(dsa);
dsh_state->dsh_handle = dshash_get_hash_table_handle(ret);
+
+ entry->initialized = true;
}
else if (entry->type != DSMR_ENTRY_TYPE_DSH)
ereport(ERROR,
- (errmsg("requested DSHash does not match type of existing entry")));
+ (errmsg("requested DSHash \"%s\" does not match type of existing entry",
+ name)));
+ else if (!entry->initialized)
+ ereport(ERROR,
+ (errmsg("requested DSHash \"%s\" failed initialization",
+ name)));
else
{
NamedDSHState *dsh_state = &entry->dsh;
--
2.39.5 (Apple Git-154)
From bc876604668340706ead2dc64db2b692f0658618 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 10 Nov 2025 12:03:47 -0600
Subject: [PATCH v3 2/2] test_dsa: Avoid leaking LWLock tranches.
---
src/test/modules/test_dsa/test_dsa.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c
index 01d5c6fa67f..21e4ce6a745 100644
--- a/src/test/modules/test_dsa/test_dsa.c
+++ b/src/test/modules/test_dsa/test_dsa.c
@@ -13,25 +13,35 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/dsm_registry.h"
#include "storage/lwlock.h"
#include "utils/dsa.h"
#include "utils/resowner.h"
PG_MODULE_MAGIC;
+static void
+init_tranche(void *ptr)
+{
+ int *tranche_id = (int *) ptr;
+
+ *tranche_id = LWLockNewTrancheId("test_dsa");
+}
+
/* Test basic DSA functionality */
PG_FUNCTION_INFO_V1(test_dsa_basic);
Datum
test_dsa_basic(PG_FUNCTION_ARGS)
{
- int tranche_id;
+ int *tranche_id;
+ bool found;
dsa_area *a;
dsa_pointer p[100];
- /* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId("test_dsa");
+ tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
+ init_tranche, &found);
- a = dsa_create(tranche_id);
+ a = dsa_create(*tranche_id);
for (int i = 0; i < 100; i++)
{
p[i] = dsa_allocate(a, 1000);
@@ -62,17 +72,18 @@ PG_FUNCTION_INFO_V1(test_dsa_resowners);
Datum
test_dsa_resowners(PG_FUNCTION_ARGS)
{
- int tranche_id;
+ int *tranche_id;
+ bool found;
dsa_area *a;
dsa_pointer p[10000];
ResourceOwner oldowner;
ResourceOwner childowner;
- /* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId("test_dsa");
+ tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
+ init_tranche, &found);
/* Create DSA in parent resource owner */
- a = dsa_create(tranche_id);
+ a = dsa_create(*tranche_id);
/*
* Switch to child resource owner, and do a bunch of allocations in the
--
2.39.5 (Apple Git-154)
Attachments:
v3-0001-DSM-registry-ERROR-if-entry-was-not-initialized.patch (text/plain, 3.6K) download
From 0b12db92e17dd315cc08b5a918196b685e3f6f61 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 10 Nov 2025 11:43:51 -0600
Subject: [PATCH v3 1/2] DSM registry: ERROR if entry was not initialized.
---
src/backend/storage/ipc/dsm_registry.c | 34 +++++++++++++++++++++++---
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index a926b9c3f32..7eba8a8cffb 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -93,6 +93,7 @@ typedef struct DSMRegistryEntry
{
char name[NAMEDATALEN];
DSMREntryType type;
+ bool initialized;
union
{
NamedDSMState dsm;
@@ -216,6 +217,7 @@ GetNamedDSMSegment(const char *name, size_t size,
dsm_segment *seg;
entry->type = DSMR_ENTRY_TYPE_DSM;
+ entry->initialized = false;
/* Initialize the segment. */
seg = dsm_create(size, 0);
@@ -228,13 +230,21 @@ GetNamedDSMSegment(const char *name, size_t size,
if (init_callback)
(*init_callback) (ret);
+
+ entry->initialized = true;
}
else if (entry->type != DSMR_ENTRY_TYPE_DSM)
ereport(ERROR,
- (errmsg("requested DSM segment does not match type of existing entry")));
+ (errmsg("requested DSM segment \"%s\" does not match type of existing entry",
+ name)));
+ else if (!entry->initialized)
+ ereport(ERROR,
+ (errmsg("requested DSM segment \"%s\" failed initialization",
+ name)));
else if (entry->dsm.size != size)
ereport(ERROR,
- (errmsg("requested DSM segment size does not match size of existing segment")));
+ (errmsg("requested DSM segment \"%s\" does not match size of existing entry",
+ name)));
else
{
NamedDSMState *state = &entry->dsm;
@@ -297,6 +307,7 @@ GetNamedDSA(const char *name, bool *found)
NamedDSAState *state = &entry->dsa;
entry->type = DSMR_ENTRY_TYPE_DSA;
+ entry->initialized = false;
/* Initialize the LWLock tranche for the DSA. */
state->tranche = LWLockNewTrancheId(name);
@@ -308,10 +319,17 @@ GetNamedDSA(const char *name, bool *found)
/* Store handle for other backends to use. */
state->handle = dsa_get_handle(ret);
+
+ entry->initialized = true;
}
else if (entry->type != DSMR_ENTRY_TYPE_DSA)
ereport(ERROR,
- (errmsg("requested DSA does not match type of existing entry")));
+ (errmsg("requested DSA \"%s\" does not match type of existing entry",
+ name)));
+ else if (!entry->initialized)
+ ereport(ERROR,
+ (errmsg("requested DSA \"%s\" failed initialization",
+ name)));
else
{
NamedDSAState *state = &entry->dsa;
@@ -372,6 +390,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
dsa_area *dsa;
entry->type = DSMR_ENTRY_TYPE_DSH;
+ entry->initialized = false;
/* Initialize the LWLock tranche for the hash table. */
dsh_state->tranche = LWLockNewTrancheId(name);
@@ -389,10 +408,17 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
/* Store handles for other backends to use. */
dsh_state->dsa_handle = dsa_get_handle(dsa);
dsh_state->dsh_handle = dshash_get_hash_table_handle(ret);
+
+ entry->initialized = true;
}
else if (entry->type != DSMR_ENTRY_TYPE_DSH)
ereport(ERROR,
- (errmsg("requested DSHash does not match type of existing entry")));
+ (errmsg("requested DSHash \"%s\" does not match type of existing entry",
+ name)));
+ else if (!entry->initialized)
+ ereport(ERROR,
+ (errmsg("requested DSHash \"%s\" failed initialization",
+ name)));
else
{
NamedDSHState *dsh_state = &entry->dsh;
--
2.39.5 (Apple Git-154)
v3-0002-test_dsa-Avoid-leaking-LWLock-tranches.patch (text/plain, 2.0K) download
From bc876604668340706ead2dc64db2b692f0658618 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 10 Nov 2025 12:03:47 -0600
Subject: [PATCH v3 2/2] test_dsa: Avoid leaking LWLock tranches.
---
src/test/modules/test_dsa/test_dsa.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c
index 01d5c6fa67f..21e4ce6a745 100644
--- a/src/test/modules/test_dsa/test_dsa.c
+++ b/src/test/modules/test_dsa/test_dsa.c
@@ -13,25 +13,35 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/dsm_registry.h"
#include "storage/lwlock.h"
#include "utils/dsa.h"
#include "utils/resowner.h"
PG_MODULE_MAGIC;
+static void
+init_tranche(void *ptr)
+{
+ int *tranche_id = (int *) ptr;
+
+ *tranche_id = LWLockNewTrancheId("test_dsa");
+}
+
/* Test basic DSA functionality */
PG_FUNCTION_INFO_V1(test_dsa_basic);
Datum
test_dsa_basic(PG_FUNCTION_ARGS)
{
- int tranche_id;
+ int *tranche_id;
+ bool found;
dsa_area *a;
dsa_pointer p[100];
- /* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId("test_dsa");
+ tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
+ init_tranche, &found);
- a = dsa_create(tranche_id);
+ a = dsa_create(*tranche_id);
for (int i = 0; i < 100; i++)
{
p[i] = dsa_allocate(a, 1000);
@@ -62,17 +72,18 @@ PG_FUNCTION_INFO_V1(test_dsa_resowners);
Datum
test_dsa_resowners(PG_FUNCTION_ARGS)
{
- int tranche_id;
+ int *tranche_id;
+ bool found;
dsa_area *a;
dsa_pointer p[10000];
ResourceOwner oldowner;
ResourceOwner childowner;
- /* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId("test_dsa");
+ tranche_id = GetNamedDSMSegment("test_dsa", sizeof(int),
+ init_tranche, &found);
/* Create DSA in parent resource owner */
- a = dsa_create(tranche_id);
+ a = dsa_create(*tranche_id);
/*
* Switch to child resource owner, and do a bunch of allocations in the
--
2.39.5 (Apple Git-154)
view thread (107+ messages) latest in thread
Message-ID: <aROOSLQwamg339K-@nathan>
Permalink: ../aROOSLQwamg339K-@nathan/
Also on: postgresql.org/message-id/aROOSLQwamg339K-@nathan
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, samimseih@gmail.com, exclusion@gmail.com, rahilasyed90@gmail.com, bertranddrouvot.pg@gmail.com, tgl@sss.pgh.pa.us, andres@anarazel.de
Subject: Re: Improve LWLock tranche name visibility across backends
In-Reply-To: <aROOSLQwamg339K-@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox