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: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: Rahila Syed <rahilasyed90@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Improve LWLock tranche name visibility across backends
Date: Sat, 30 Aug 2025 09:14:46 -0500
Message-ID: <aLMHVosexT5D-Ybz@nathan> (raw )
In-Reply-To: <aLJnOnTFSweb2pud@nathan >
References: <aK4Vr0puSQqPI5vS@nathan >
<CAA5RZ0v=vvzn4mvejQVA1iFabxSTUQZPDuTCAF0WqA41LKns7g@mail.gmail.com >
<aK7PyCbPib+/JO2+@ip-10-97-1-34.eu-west-3.compute.internal >
<CAA5RZ0ve-fHuNYW-ruMwg1y1v7-aCqMm_MiNq1KOdg2Y2-pKDw@mail.gmail.com >
<aLAFhwpzEwbjiWLG@ip-10-97-1-34.eu-west-3.compute.internal >
<CAA5RZ0s4ip08Lsk=aYuXGCkBr78jhH=gOau939mhAp-rGuNC3Q@mail.gmail.com >
<aLCEo0ZMV3vIGRWk@nathan >
<CAA5RZ0vw-r1qjHw5hUNTzAyCiK3KQ-r+G5ECfWSn+qjdu-fpnw@mail.gmail.com >
<aLHO-jh-spXWG1pt@nathan >
<aLJnOnTFSweb2pud@nathan >
On Fri, Aug 29, 2025 at 09:51:38PM -0500, Nathan Bossart wrote:
> I've also attached a rebased patch that addresses all the latest feedback.
> A reworked verison of the test patch is also included, but that's mostly
> intended for CI purposes and is still not intended for commit (yet).
And here's an attempt at fixing the alignment problems revealed by cfbot.
--
nathan
From 72260a5f2fd93c77927a50aaf521c9f294d58ac7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 27 Aug 2025 15:13:41 -0500
Subject: [PATCH v20 1/2] Move dynamically-allocated tranche names to shared
memory.
---
contrib/pg_prewarm/autoprewarm.c | 3 +-
doc/src/sgml/xfunc.sgml | 15 +-
src/backend/postmaster/launch_backend.c | 6 +-
src/backend/storage/ipc/dsm_registry.c | 12 +-
src/backend/storage/lmgr/lwlock.c | 206 +++++++++---------
src/include/storage/lwlock.h | 24 +-
src/test/modules/test_dsa/test_dsa.c | 6 +-
.../test_dsm_registry/test_dsm_registry.c | 3 +-
.../modules/test_radixtree/test_radixtree.c | 9 +-
src/test/modules/test_slru/test_slru.c | 6 +-
.../modules/test_tidstore/test_tidstore.c | 3 +-
11 files changed, 120 insertions(+), 173 deletions(-)
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index c01b9c7e6a4..880e897796a 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -864,7 +864,7 @@ apw_init_state(void *ptr)
{
AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr;
- LWLockInitialize(&state->lock, LWLockNewTrancheId());
+ LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm"));
state->bgworker_pid = InvalidPid;
state->pid_using_dumpfile = InvalidPid;
}
@@ -883,7 +883,6 @@ apw_init_shmem(void)
sizeof(AutoPrewarmSharedState),
apw_init_state,
&found);
- LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm");
return found;
}
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index f116d0648e5..da21ef56891 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name)
<literal>shmem_request_hook</literal>. To do so, first allocate a
<literal>tranche_id</literal> by calling:
<programlisting>
-int LWLockNewTrancheId(void)
+int LWLockNewTrancheId(const char *name)
</programlisting>
Next, initialize each LWLock, passing the new
<literal>tranche_id</literal> as an argument:
@@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id)
</para>
<para>
- Finally, each backend using the <literal>tranche_id</literal> should
- associate it with a <literal>tranche_name</literal> by calling:
-<programlisting>
-void LWLockRegisterTranche(int tranche_id, const char *tranche_name)
-</programlisting>
- </para>
-
- <para>
- A complete usage example of <function>LWLockNewTrancheId</function>,
- <function>LWLockInitialize</function>, and
- <function>LWLockRegisterTranche</function> can be found in
+ A complete usage example of <function>LWLockNewTrancheId</function> and
+ <function>LWLockInitialize</function> can be found in
<filename>contrib/pg_prewarm/autoprewarm.c</filename> in the
<productname>PostgreSQL</productname> source tree.
</para>
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index cd9547b03a3..de1b06df10d 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -101,7 +101,7 @@ typedef struct
struct InjectionPointsCtl *ActiveInjectionPoints;
#endif
int NamedLWLockTrancheRequests;
- NamedLWLockTranche *NamedLWLockTrancheArray;
+ char **NamedLWLockTrancheNames;
int *LWLockCounter;
LWLockPadded *MainLWLockArray;
slock_t *ProcStructLock;
@@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param,
#endif
param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests;
- param->NamedLWLockTrancheArray = NamedLWLockTrancheArray;
+ param->NamedLWLockTrancheNames = NamedLWLockTrancheNames;
param->LWLockCounter = LWLockCounter;
param->MainLWLockArray = MainLWLockArray;
param->ProcStructLock = ProcStructLock;
@@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param)
#endif
NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests;
- NamedLWLockTrancheArray = param->NamedLWLockTrancheArray;
+ NamedLWLockTrancheNames = param->NamedLWLockTrancheNames;
LWLockCounter = param->LWLockCounter;
MainLWLockArray = param->MainLWLockArray;
ProcStructLock = param->ProcStructLock;
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index ca12815f4a8..97130925106 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found)
entry->type = DSMR_ENTRY_TYPE_DSA;
/* Initialize the LWLock tranche for the DSA. */
- state->tranche = LWLockNewTrancheId();
- LWLockRegisterTranche(state->tranche, name);
+ state->tranche = LWLockNewTrancheId(name);
/* Initialize the DSA. */
ret = dsa_create(state->tranche);
@@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found)
ereport(ERROR,
(errmsg("requested DSA already attached to current process")));
- /* Initialize existing LWLock tranche for the DSA. */
- LWLockRegisterTranche(state->tranche, name);
-
/* Attach to existing DSA. */
ret = dsa_attach(state->handle);
dsa_pin_mapping(ret);
@@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
entry->type = DSMR_ENTRY_TYPE_DSH;
/* Initialize the LWLock tranche for the hash table. */
- dsh_state->tranche = LWLockNewTrancheId();
- LWLockRegisterTranche(dsh_state->tranche, name);
+ dsh_state->tranche = LWLockNewTrancheId(name);
/* Initialize the DSA for the hash table. */
dsa = dsa_create(dsh_state->tranche);
@@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
ereport(ERROR,
(errmsg("requested DSHash already attached to current process")));
- /* Initialize existing LWLock tranche for the hash table. */
- LWLockRegisterTranche(dsh_state->tranche, name);
-
/* Attach to existing DSA for the hash table. */
dsa = dsa_attach(dsh_state->dsa_handle);
dsa_pin_mapping(dsa);
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index a4aecd1fbc3..b4636d86ca6 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0,
* in lwlocklist.h. We absorb the names of these tranches, too.
*
* 3. Extensions can create new tranches, via either RequestNamedLWLockTranche
- * or LWLockRegisterTranche. The names of these that are known in the current
- * process appear in LWLockTrancheNames[].
+ * or LWLockNewTrancheId. These names are stored in shared memory and can be
+ * accessed via NamedLWLockTrancheNames.
*
* All these names are user-visible as wait event names, so choose with care
* ... and do not forget to update the documentation's list of wait events.
@@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) ==
/*
* This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and
- * stores the names of all dynamically-created tranches known to the current
- * process. Any unused entries in the array will contain NULL.
+ * points to the shared memory locations of the names of all
+ * dynamically-created tranches. Backends inherit the pointer by fork from the
+ * postmaster (except in the EXEC_BACKEND case, where we have special measures
+ * to pass it down).
*/
-static const char **LWLockTrancheNames = NULL;
-static int LWLockTrancheNamesAllocated = 0;
+char **NamedLWLockTrancheNames = NULL;
/*
* This points to the main array of LWLocks in shared memory. Backends inherit
@@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest
} NamedLWLockTrancheRequest;
static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL;
-static int NamedLWLockTrancheRequestsAllocated = 0;
/*
- * NamedLWLockTrancheRequests is both the valid length of the request array,
- * and the length of the shared-memory NamedLWLockTrancheArray later on.
- * This variable and NamedLWLockTrancheArray are non-static so that
- * postmaster.c can copy them to child processes in EXEC_BACKEND builds.
+ * NamedLWLockTrancheRequests is the valid length of the request array. This
+ * variable is non-static so that postmaster.c can copy them to child processes
+ * in EXEC_BACKEND builds.
*/
int NamedLWLockTrancheRequests = 0;
/* points to data in shared memory: */
-NamedLWLockTranche *NamedLWLockTrancheArray = NULL;
int *LWLockCounter = NULL;
+/* backend-local counter of registered tranches */
+static int LocalLWLockCounter;
+
+#define MAX_NAMED_TRANCHES 256
+
static void InitializeLWLocks(void);
static inline void LWLockReportWaitStart(LWLock *lock);
static inline void LWLockReportWaitEnd(void);
@@ -392,31 +395,28 @@ Size
LWLockShmemSize(void)
{
Size size;
- int i;
int numLocks = NUM_FIXED_LWLOCKS;
/* Calculate total number of locks needed in the main array. */
numLocks += NumLWLocksForNamedTranches();
- /* Space for dynamic allocation counter, plus room for alignment. */
- size = sizeof(int) + LWLOCK_PADDED_SIZE;
+ /* Space for dynamic allocation counter. */
+ size = MAXALIGN(sizeof(int));
- /* Space for the LWLock array. */
- size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded)));
+ /* Space for named tranches. */
+ size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *)));
+ size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN));
- /* space for named tranches. */
- size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche)));
-
- /* space for name of each tranche. */
- for (i = 0; i < NamedLWLockTrancheRequests; i++)
- size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1);
+ /* Space for the LWLock array, plus room for cache line alignment. */
+ size = add_size(size, LWLOCK_PADDED_SIZE);
+ size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded)));
return size;
}
/*
* Allocate shmem space for the main LWLock array and all tranches and
- * initialize it. We also register extension LWLock tranches here.
+ * initialize it.
*/
void
CreateLWLocks(void)
@@ -432,7 +432,16 @@ CreateLWLocks(void)
/* Initialize the dynamic-allocation counter for tranches */
LWLockCounter = (int *) ptr;
*LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED;
- ptr += sizeof(int);
+ ptr += MAXALIGN(sizeof(int));
+
+ /* Initialize tranche names */
+ NamedLWLockTrancheNames = (char **) ptr;
+ ptr += MAX_NAMED_TRANCHES * sizeof(char *);
+ for (int i = 0; i < MAX_NAMED_TRANCHES; i++)
+ {
+ NamedLWLockTrancheNames[i] = ptr;
+ ptr += NAMEDATALEN;
+ }
/* Ensure desired alignment of LWLock array */
ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE;
@@ -441,11 +450,6 @@ CreateLWLocks(void)
/* Initialize all LWLocks */
InitializeLWLocks();
}
-
- /* Register named extension LWLock tranches in the current process. */
- for (int i = 0; i < NamedLWLockTrancheRequests; i++)
- LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId,
- NamedLWLockTrancheArray[i].trancheName);
}
/*
@@ -454,7 +458,6 @@ CreateLWLocks(void)
static void
InitializeLWLocks(void)
{
- int numNamedLocks = NumLWLocksForNamedTranches();
int id;
int i;
int j;
@@ -485,32 +488,18 @@ InitializeLWLocks(void)
*/
if (NamedLWLockTrancheRequests > 0)
{
- char *trancheNames;
-
- NamedLWLockTrancheArray = (NamedLWLockTranche *)
- &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks];
-
- trancheNames = (char *) NamedLWLockTrancheArray +
- (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche));
lock = &MainLWLockArray[NUM_FIXED_LWLOCKS];
for (i = 0; i < NamedLWLockTrancheRequests; i++)
{
NamedLWLockTrancheRequest *request;
- NamedLWLockTranche *tranche;
- char *name;
+ int tranche;
request = &NamedLWLockTrancheRequestArray[i];
- tranche = &NamedLWLockTrancheArray[i];
-
- name = trancheNames;
- trancheNames += strlen(request->tranche_name) + 1;
- strcpy(name, request->tranche_name);
- tranche->trancheId = LWLockNewTrancheId();
- tranche->trancheName = name;
+ tranche = LWLockNewTrancheId(request->tranche_name);
for (j = 0; j < request->num_lwlocks; j++, lock++)
- LWLockInitialize(&lock->lock, tranche->trancheId);
+ LWLockInitialize(&lock->lock, tranche);
}
}
}
@@ -562,59 +551,44 @@ GetNamedLWLockTranche(const char *tranche_name)
}
/*
- * Allocate a new tranche ID.
+ * Allocate a new tranche ID with the provided name.
*/
int
-LWLockNewTrancheId(void)
+LWLockNewTrancheId(const char *name)
{
int result;
- /* We use the ShmemLock spinlock to protect LWLockCounter */
- SpinLockAcquire(ShmemLock);
- result = (*LWLockCounter)++;
- SpinLockRelease(ShmemLock);
+ if (!name)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("tranche name cannot be NULL")));
- return result;
-}
+ if (strlen(name) >= NAMEDATALEN)
+ ereport(ERROR,
+ (errcode(ERRCODE_NAME_TOO_LONG),
+ errmsg("tranche name too long"),
+ errdetail("LWLock tranche names must be no longer than %d bytes.",
+ NAMEDATALEN - 1)));
-/*
- * Register a dynamic tranche name in the lookup table of the current process.
- *
- * This routine will save a pointer to the tranche name passed as an argument,
- * so the name should be allocated in a backend-lifetime context
- * (shared memory, TopMemoryContext, static constant, or similar).
- *
- * The tranche name will be user-visible as a wait event name, so try to
- * use a name that fits the style for those.
- */
-void
-LWLockRegisterTranche(int tranche_id, const char *tranche_name)
-{
- /* This should only be called for user-defined tranches. */
- if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED)
- return;
-
- /* Convert to array index. */
- tranche_id -= LWTRANCHE_FIRST_USER_DEFINED;
+ /* We use the ShmemLock spinlock to protect LWLockCounter */
+ SpinLockAcquire(ShmemLock);
- /* If necessary, create or enlarge array. */
- if (tranche_id >= LWLockTrancheNamesAllocated)
+ if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES)
{
- int newalloc;
+ SpinLockRelease(ShmemLock);
+ ereport(ERROR,
+ (errmsg("maximum number of tranches already registered"),
+ errdetail("No more than %d tranches may be registered.",
+ MAX_NAMED_TRANCHES)));
+ }
- newalloc = pg_nextpower2_32(Max(8, tranche_id + 1));
+ result = (*LWLockCounter)++;
+ LocalLWLockCounter = *LWLockCounter;
+ strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN);
- if (LWLockTrancheNames == NULL)
- LWLockTrancheNames = (const char **)
- MemoryContextAllocZero(TopMemoryContext,
- newalloc * sizeof(char *));
- else
- LWLockTrancheNames =
- repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc);
- LWLockTrancheNamesAllocated = newalloc;
- }
+ SpinLockRelease(ShmemLock);
- LWLockTrancheNames[tranche_id] = tranche_name;
+ return result;
}
/*
@@ -637,27 +611,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
if (!process_shmem_requests_in_progress)
elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook");
+ if (!tranche_name)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("tranche name cannot be NULL")));
+
+ if (strlen(tranche_name) >= NAMEDATALEN)
+ ereport(ERROR,
+ (errcode(ERRCODE_NAME_TOO_LONG),
+ errmsg("tranche name too long"),
+ errdetail("LWLock tranche names must be no longer than %d bytes.",
+ NAMEDATALEN - 1)));
+
if (NamedLWLockTrancheRequestArray == NULL)
{
- NamedLWLockTrancheRequestsAllocated = 16;
NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *)
MemoryContextAlloc(TopMemoryContext,
- NamedLWLockTrancheRequestsAllocated
+ MAX_NAMED_TRANCHES
* sizeof(NamedLWLockTrancheRequest));
}
- if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated)
- {
- int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1);
-
- NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *)
- repalloc(NamedLWLockTrancheRequestArray,
- i * sizeof(NamedLWLockTrancheRequest));
- NamedLWLockTrancheRequestsAllocated = i;
- }
+ if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES)
+ ereport(ERROR,
+ (errmsg("maximum number of tranches already registered"),
+ errdetail("No more than %d tranches may be registered.",
+ MAX_NAMED_TRANCHES)));
request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests];
- Assert(strlen(tranche_name) + 1 <= NAMEDATALEN);
strlcpy(request->tranche_name, tranche_name, NAMEDATALEN);
request->num_lwlocks = num_lwlocks;
NamedLWLockTrancheRequests++;
@@ -669,6 +649,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
void
LWLockInitialize(LWLock *lock, int tranche_id)
{
+ /* verify the tranche_id is valid */
+ (void) GetLWTrancheName(tranche_id);
+
pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK);
#ifdef LOCK_DEBUG
pg_atomic_init_u32(&lock->nwaiters, 0);
@@ -709,18 +692,23 @@ GetLWTrancheName(uint16 trancheId)
if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
return BuiltinTrancheNames[trancheId];
+ /* verify the trancheId is valid */
+ if (trancheId >= LocalLWLockCounter)
+ {
+ SpinLockAcquire(ShmemLock);
+ LocalLWLockCounter = *LWLockCounter;
+ SpinLockRelease(ShmemLock);
+
+ if (trancheId >= LocalLWLockCounter)
+ elog(ERROR, "tranche %d is not registered", trancheId);
+ }
+
/*
- * It's an extension tranche, so look in LWLockTrancheNames[]. However,
- * it's possible that the tranche has never been registered in the current
- * process, in which case give up and return "extension".
+ * It's an extension tranche, so look in NamedLWLockTrancheNames.
*/
trancheId -= LWTRANCHE_FIRST_USER_DEFINED;
- if (trancheId >= LWLockTrancheNamesAllocated ||
- LWLockTrancheNames[trancheId] == NULL)
- return "extension";
-
- return LWLockTrancheNames[trancheId];
+ return NamedLWLockTrancheNames[trancheId];
}
/*
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index f9cf57f8d26..3877aaa7f03 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -73,14 +73,7 @@ typedef union LWLockPadded
extern PGDLLIMPORT LWLockPadded *MainLWLockArray;
-/* struct for storing named tranche information */
-typedef struct NamedLWLockTranche
-{
- int trancheId;
- char *trancheName;
-} NamedLWLockTranche;
-
-extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray;
+extern PGDLLIMPORT char **NamedLWLockTrancheNames;
extern PGDLLIMPORT int NamedLWLockTrancheRequests;
extern PGDLLIMPORT int *LWLockCounter;
@@ -158,18 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name);
/*
* There is another, more flexible method of obtaining lwlocks. First, call
- * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from
- * a shared counter. Next, each individual process using the tranche should
- * call LWLockRegisterTranche() to associate that tranche ID with a name.
- * Finally, LWLockInitialize should be called just once per lwlock, passing
- * the tranche ID as an argument.
- *
- * It may seem strange that each process using the tranche must register it
- * separately, but dynamic shared memory segments aren't guaranteed to be
- * mapped at the same address in all coordinating backends, so storing the
- * registration in the main shared memory segment wouldn't work for that case.
+ * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared
+ * counter. Second, LWLockInitialize should be called just once per lwlock,
+ * passing the tranche ID as an argument.
*/
-extern int LWLockNewTrancheId(void);
+extern int LWLockNewTrancheId(const char *name);
extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name);
extern void LWLockInitialize(LWLock *lock, int tranche_id);
diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c
index cd24d0f4873..01d5c6fa67f 100644
--- a/src/test/modules/test_dsa/test_dsa.c
+++ b/src/test/modules/test_dsa/test_dsa.c
@@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS)
dsa_pointer p[100];
/* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(tranche_id, "test_dsa");
+ tranche_id = LWLockNewTrancheId("test_dsa");
a = dsa_create(tranche_id);
for (int i = 0; i < 100; i++)
@@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS)
ResourceOwner childowner;
/* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(tranche_id, "test_dsa");
+ tranche_id = LWLockNewTrancheId("test_dsa");
/* Create DSA in parent resource owner */
a = dsa_create(tranche_id);
diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c
index 141c8ed1b34..4cc2ccdac3f 100644
--- a/src/test/modules/test_dsm_registry/test_dsm_registry.c
+++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c
@@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr)
{
TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr;
- LWLockInitialize(&dsm->lck, LWLockNewTrancheId());
+ LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry"));
dsm->val = 0;
}
@@ -61,7 +61,6 @@ tdr_attach_shmem(void)
sizeof(TestDSMRegistryStruct),
init_tdr_dsm,
&found);
- LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry");
if (tdr_dsa == NULL)
tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found);
diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c
index 80ad0296164..787162c8793 100644
--- a/src/test/modules/test_radixtree/test_radixtree.c
+++ b/src/test/modules/test_radixtree/test_radixtree.c
@@ -124,10 +124,9 @@ test_empty(void)
rt_iter *iter;
uint64 key;
#ifdef TEST_SHARED_RT
- int tranche_id = LWLockNewTrancheId();
+ int tranche_id = LWLockNewTrancheId("test_radix_tree");
dsa_area *dsa;
- LWLockRegisterTranche(tranche_id, "test_radix_tree");
dsa = dsa_create(tranche_id);
radixtree = rt_create(dsa, tranche_id);
#else
@@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc)
uint64 *keys;
int children = test_info->nkeys;
#ifdef TEST_SHARED_RT
- int tranche_id = LWLockNewTrancheId();
+ int tranche_id = LWLockNewTrancheId("test_radix_tree");
dsa_area *dsa;
- LWLockRegisterTranche(tranche_id, "test_radix_tree");
dsa = dsa_create(tranche_id);
radixtree = rt_create(dsa, tranche_id);
#else
@@ -304,10 +302,9 @@ test_random(void)
int num_keys = 100000;
uint64 *keys;
#ifdef TEST_SHARED_RT
- int tranche_id = LWLockNewTrancheId();
+ int tranche_id = LWLockNewTrancheId("test_radix_tree");
dsa_area *dsa;
- LWLockRegisterTranche(tranche_id, "test_radix_tree");
dsa = dsa_create(tranche_id);
radixtree = rt_create(dsa, tranche_id);
#else
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 32750930e43..8c0367eeee4 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -232,11 +232,9 @@ test_slru_shmem_startup(void)
(void) MakePGDirectory(slru_dir_name);
/* initialize the SLRU facility */
- test_tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(test_tranche_id, "test_slru_tranche");
+ test_tranche_id = LWLockNewTrancheId("test_slru_tranche");
- test_buffer_tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche");
+ test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche");
TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
SimpleLruInit(TestSlruCtl, "TestSLRU",
diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c
index eb16e0fbfa6..0c8f43867e5 100644
--- a/src/test/modules/test_tidstore/test_tidstore.c
+++ b/src/test/modules/test_tidstore/test_tidstore.c
@@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS)
{
int tranche_id;
- tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(tranche_id, "test_tidstore");
+ tranche_id = LWLockNewTrancheId("test_tidstore");
tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id);
--
2.39.5 (Apple Git-154)
From 65c80071e74c36328873dfa9cf6ce8f537d83850 Mon Sep 17 00:00:00 2001
From: Sami Imseih <simseih@amazon.com>
Date: Fri, 22 Aug 2025 00:35:45 -0500
Subject: [PATCH v20 2/2] Tests for LWLock tranche registration improvements
---
src/test/modules/Makefile | 3 +-
src/test/modules/meson.build | 1 +
src/test/modules/test_tranches/.gitignore | 4 +
src/test/modules/test_tranches/Makefile | 23 +++
src/test/modules/test_tranches/meson.build | 33 +++
.../test_tranches/t/001_test_tranches.pl | 120 +++++++++++
.../test_tranches/test_tranches--1.0.sql | 35 ++++
.../modules/test_tranches/test_tranches.c | 191 ++++++++++++++++++
.../test_tranches/test_tranches.control | 6 +
9 files changed, 415 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/test_tranches/.gitignore
create mode 100644 src/test/modules/test_tranches/Makefile
create mode 100644 src/test/modules/test_tranches/meson.build
create mode 100644 src/test/modules/test_tranches/t/001_test_tranches.pl
create mode 100644 src/test/modules/test_tranches/test_tranches--1.0.sql
create mode 100644 src/test/modules/test_tranches/test_tranches.c
create mode 100644 src/test/modules/test_tranches/test_tranches.control
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 903a8ac151a..fd7aa4675c5 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -44,7 +44,8 @@ SUBDIRS = \
test_tidstore \
unsafe_tests \
worker_spi \
- xid_wraparound
+ xid_wraparound \
+ test_tranches
ifeq ($(enable_injection_points),yes)
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 93be0f57289..52883dfb496 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -45,3 +45,4 @@ subdir('typcache')
subdir('unsafe_tests')
subdir('worker_spi')
subdir('xid_wraparound')
+subdir('test_tranches')
diff --git a/src/test/modules/test_tranches/.gitignore b/src/test/modules/test_tranches/.gitignore
new file mode 100644
index 00000000000..5dcb3ff9723
--- /dev/null
+++ b/src/test/modules/test_tranches/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_tranches/Makefile b/src/test/modules/test_tranches/Makefile
new file mode 100644
index 00000000000..5c9e78084da
--- /dev/null
+++ b/src/test/modules/test_tranches/Makefile
@@ -0,0 +1,23 @@
+# src/test/modules/test_tranches/Makefile
+
+MODULE_big = test_tranches
+OBJS = \
+ $(WIN32RES) \
+ test_tranches.o
+PGFILEDESC = "test_tranches - test code LWLock tranche management"
+
+EXTENSION = test_tranches
+DATA = test_tranches--1.0.sql
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_tranches
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_tranches/meson.build b/src/test/modules/test_tranches/meson.build
new file mode 100644
index 00000000000..976ce9f0a25
--- /dev/null
+++ b/src/test/modules/test_tranches/meson.build
@@ -0,0 +1,33 @@
+# Copyright (c) 2024-2025, PostgreSQL Global Development Group
+
+test_tranches_sources = files(
+ 'test_tranches.c',
+)
+
+if host_system == 'windows'
+ test_tranches_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+ '--NAME', 'test_tranches',
+ '--FILEDESC', 'test_tranches - test code LWLock tranche management',])
+endif
+
+test_tranches = shared_module('test_tranches',
+ test_tranches_sources,
+ kwargs: pg_test_mod_args,
+)
+test_install_libs += test_tranches
+
+test_install_data += files(
+ 'test_tranches.control',
+ 'test_tranches--1.0.sql',
+)
+
+tests += {
+ 'name': 'test_tranches',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_test_tranches.pl',
+ ],
+ },
+}
\ No newline at end of file
diff --git a/src/test/modules/test_tranches/t/001_test_tranches.pl b/src/test/modules/test_tranches/t/001_test_tranches.pl
new file mode 100644
index 00000000000..477035bdb99
--- /dev/null
+++ b/src/test/modules/test_tranches/t/001_test_tranches.pl
@@ -0,0 +1,120 @@
+use strict;
+use warnings FATAL => 'all';
+
+use List::Util qw(min);
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+#
+# Create the cluster without any requested tranches
+#
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init();
+$node->append_conf('postgresql.conf',
+ qq(shared_preload_libraries='test_tranches'));
+$node->append_conf('postgresql.conf',
+ qq(test_tranches.requested_named_tranches=0));
+$node->start();
+$node->safe_psql('postgres', q(CREATE EXTENSION test_tranches));
+
+my $first_user_defined = $node->safe_psql('postgres', q(select test_tranches_get_first_user_defined()));
+my @user_defined = ($first_user_defined .. $first_user_defined + 5);
+my ($second_user_defined,
+ $third_user_defined,
+ $fourth_user_defined,
+ $fifth_user_defined,
+ $sixth_user_defined) = @user_defined[1..5];
+#
+# Run lookup tests to ensure the correct tranche names are returned
+# and an error is raised for unregistered tranches
+#
+$node->safe_psql('postgres', "select test_tranches_new(5);");
+my ($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{
+ select test_tranches_lookup(1);
+ select test_tranches_lookup($first_user_defined);
+ select test_tranches_lookup($second_user_defined);
+ select test_tranches_lookup($third_user_defined);
+ select test_tranches_lookup($fourth_user_defined);
+ select test_tranches_lookup($fifth_user_defined);
+ select test_tranches_lookup($sixth_user_defined);
+ },
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche 100 is not registered/, "unregistered tranche error");
+like("$stdout",
+ qr/ShmemIndex.*test_lock__0.*test_lock__1.*test_lock__2.*test_lock__3.*test_lock__4/s,
+ "match tranche names without requested tranches");
+
+#
+# Test the error for long tranche names
+#
+my $good_tranche_name = 'A' x 63;
+my $bad_tranche_name = 'B' x 64; # MAX_NAMED_TRANCHES_NAME_LEN
+$node->safe_psql('postgres', qq{select test_tranches_new_tranche('$good_tranche_name');});
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{
+ select test_tranches_new_tranche('$bad_tranche_name');
+ },
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche name too long/, "tranche name too long");
+
+$node->restart();
+
+#
+# Test the error when > MAX_NAMED_TRANCHES named tranches registered.
+#
+$node->safe_psql('postgres', qq{select test_tranches_new(255)});
+$node->safe_psql('postgres', qq{select test_tranches_new(1)});
+($result, $stdout, $stderr) = $node->psql('postgres', qq{select test_tranches_new(1);}, on_error_stop => 0);
+like("$stderr", qr/ERROR: maximum number of tranches already registered/, "too many tranches registered");
+
+#
+# Repeat the lookup test with 2 requested tranches
+#
+$node->append_conf('postgresql.conf',
+ qq(test_tranches.requested_named_tranches=2));
+$node->restart();
+
+$first_user_defined = $first_user_defined;
+@user_defined = ($first_user_defined .. $first_user_defined + 5);
+($second_user_defined,
+ $third_user_defined,
+ $fourth_user_defined,
+ $fifth_user_defined,
+ $sixth_user_defined) = @user_defined[1..5];
+
+$node->safe_psql('postgres', "select test_tranches_new(3);");
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{
+ select test_tranches_lookup(1);
+ select test_tranches_lookup($first_user_defined);
+ select test_tranches_lookup($second_user_defined);
+ select test_tranches_lookup($third_user_defined);
+ select test_tranches_lookup($fourth_user_defined);
+ select test_tranches_lookup($fifth_user_defined);
+ select test_tranches_lookup($sixth_user_defined);
+ },
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche 100 is not registered/, "unregistered tranche error");
+like("$stdout", qr/ShmemIndex.*test_lock_0.*test_lock_1.*test_lock__2.*test_lock__3.*test_lock__4/s,
+ "match tranche names with requested tranches");
+
+#
+# Test lwlock initialize
+#
+$node->safe_psql('postgres', qq{select test_tranches_lwlock_initialize($first_user_defined)});
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{select test_tranches_lwlock_initialize($sixth_user_defined)},
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche 100 is not registered/, "LWLock intialization error on invalid tranche name");
+
+#
+# Test error for NULL tranche name
+#
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{select test_tranches_new_tranche(NULL)},
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche name cannot be NULL/, "NULL tranche name");
+
+done_testing();
diff --git a/src/test/modules/test_tranches/test_tranches--1.0.sql b/src/test/modules/test_tranches/test_tranches--1.0.sql
new file mode 100644
index 00000000000..f504098e04a
--- /dev/null
+++ b/src/test/modules/test_tranches/test_tranches--1.0.sql
@@ -0,0 +1,35 @@
+-- test_tranches--1.0.sql
+
+CREATE FUNCTION test_tranches_new(bigint)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_tranches_new'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_lookup(int)
+RETURNS text
+AS 'MODULE_PATHNAME', 'test_tranches_lookup'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_get_named_lwlock(text, int)
+RETURNS int
+AS 'MODULE_PATHNAME', 'test_tranches_get_named_lwlock'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_get_first_user_defined()
+RETURNS int
+AS 'MODULE_PATHNAME', 'test_tranches_get_first_user_defined'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_lwlock_initialize(int)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_tranches_lwlock_initialize'
+LANGUAGE C STRICT;
+
+/*
+ * Function is CALLED ON NULL INPUT to allow NULL input
+ * for testing
+ */
+CREATE FUNCTION test_tranches_new_tranche(text)
+RETURNS int
+AS 'MODULE_PATHNAME', 'test_tranches_new_tranche'
+LANGUAGE C;
\ No newline at end of file
diff --git a/src/test/modules/test_tranches/test_tranches.c b/src/test/modules/test_tranches/test_tranches.c
new file mode 100644
index 00000000000..7d3031ace9f
--- /dev/null
+++ b/src/test/modules/test_tranches/test_tranches.c
@@ -0,0 +1,191 @@
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "storage/dsm_registry.h"
+#include "storage/ipc.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
+#include "utils/builtins.h"
+#include "utils/guc.h"
+#include "utils/injection_point.h"
+#include "utils/wait_classes.h"
+
+PG_MODULE_MAGIC;
+
+/* hooks */
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
+static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
+static void test_tranches_shmem_request(void);
+static void test_tranches_shmem_startup(void);
+
+/* GUC */
+static int test_tranches_requested_named_tranches = 0;
+
+typedef struct testTranchesSharedState
+{
+ int next_index;
+} testTranchesSharedState;
+
+static testTranchesSharedState * test_lwlock_ss = NULL;
+
+/*
+ * LWLock wait event masks. Copied from src/backend/utils/activity/wait_event.c
+ */
+#define WAIT_EVENT_CLASS_MASK 0xFF000000
+
+/*
+ * Module load callback
+ */
+void
+_PG_init(void)
+{
+ prev_shmem_request_hook = shmem_request_hook;
+ shmem_request_hook = test_tranches_shmem_request;
+ prev_shmem_startup_hook = shmem_startup_hook;
+ shmem_startup_hook = test_tranches_shmem_startup;
+
+ DefineCustomIntVariable("test_tranches.requested_named_tranches",
+ "Sets the number of locks created during shmem request",
+ NULL,
+ &test_tranches_requested_named_tranches,
+ 2,
+ 0,
+ UINT16_MAX,
+ PGC_POSTMASTER,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
+static void
+test_tranches_shmem_startup(void)
+{
+ bool found;
+
+ if (prev_shmem_startup_hook)
+ prev_shmem_startup_hook();
+
+ test_lwlock_ss = NULL;
+
+ test_lwlock_ss = ShmemInitStruct("test_tranches",
+ sizeof(testTranchesSharedState),
+ &found);
+ if (!found)
+ {
+ test_lwlock_ss->next_index = test_tranches_requested_named_tranches;
+ }
+}
+
+static Size
+test_tranches_memsize(void)
+{
+ Size size;
+
+ size = MAXALIGN(sizeof(testTranchesSharedState));
+
+ return size;
+}
+
+static void
+test_tranches_shmem_request(void)
+{
+ int i = 0;
+
+ if (prev_shmem_request_hook)
+ prev_shmem_request_hook();
+
+ RequestAddinShmemSpace(test_tranches_memsize());
+
+ for (i = 0; i < test_tranches_requested_named_tranches; i++)
+ {
+ char name[15];
+
+ snprintf(name, sizeof(name), "test_lock_%d", i);
+ RequestNamedLWLockTranche(name, i);
+ }
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_new);
+Datum
+test_tranches_new(PG_FUNCTION_ARGS)
+{
+ int64 num = PG_GETARG_INT64(0);
+ int i;
+
+ for (i = test_lwlock_ss->next_index; i < num + test_lwlock_ss->next_index; i++)
+ {
+ char name[50];
+
+ snprintf(name, 50, "test_lock__%d", i);
+
+ LWLockNewTrancheId(name);
+ }
+
+ test_lwlock_ss->next_index = i;
+
+ PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_new_tranche);
+Datum
+test_tranches_new_tranche(PG_FUNCTION_ARGS)
+{
+ char *tranche_name = NULL;
+
+ if (!PG_ARGISNULL(0))
+ tranche_name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+ PG_RETURN_INT32(LWLockNewTrancheId(tranche_name));
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_lookup);
+Datum
+test_tranches_lookup(PG_FUNCTION_ARGS)
+{
+ const char *tranche_name = GetLWLockIdentifier(PG_WAIT_LWLOCK & WAIT_EVENT_CLASS_MASK, PG_GETARG_INT32(0));
+
+ if (tranche_name)
+ PG_RETURN_TEXT_P(cstring_to_text(tranche_name));
+ else
+ PG_RETURN_NULL();
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_get_named_lwlock);
+Datum
+test_tranches_get_named_lwlock(PG_FUNCTION_ARGS)
+{
+ int i;
+ LWLockPadded *locks;
+
+ locks = GetNamedLWLockTranche(text_to_cstring(PG_GETARG_TEXT_PP(0)));
+
+ for (i = 0; i < PG_GETARG_INT32(1); i++)
+ {
+ LWLock *lock = &locks[i].lock;
+
+ LWLockAcquire(lock, LW_SHARED);
+ LWLockRelease(lock);
+ }
+
+ PG_RETURN_INT32(i);
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_get_first_user_defined);
+Datum
+test_tranches_get_first_user_defined(PG_FUNCTION_ARGS)
+{
+ return LWTRANCHE_FIRST_USER_DEFINED;
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_lwlock_initialize);
+Datum
+test_tranches_lwlock_initialize(PG_FUNCTION_ARGS)
+{
+ LWLock lock;
+
+ LWLockInitialize(&lock, PG_GETARG_INT32(0));
+
+ PG_RETURN_VOID();
+}
diff --git a/src/test/modules/test_tranches/test_tranches.control b/src/test/modules/test_tranches/test_tranches.control
new file mode 100644
index 00000000000..8e6254a7e43
--- /dev/null
+++ b/src/test/modules/test_tranches/test_tranches.control
@@ -0,0 +1,6 @@
+# test_tranches.control
+
+comment = 'Test LWLock tranch names tracking'
+default_version = '1.0'
+relocatable = false
+module_pathname = '$libdir/test_tranches'
\ No newline at end of file
--
2.39.5 (Apple Git-154)
Attachments:
v20-0001-Move-dynamically-allocated-tranche-names-to-shar.patch (text/plain, 24.2K) download
From 72260a5f2fd93c77927a50aaf521c9f294d58ac7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 27 Aug 2025 15:13:41 -0500
Subject: [PATCH v20 1/2] Move dynamically-allocated tranche names to shared
memory.
---
contrib/pg_prewarm/autoprewarm.c | 3 +-
doc/src/sgml/xfunc.sgml | 15 +-
src/backend/postmaster/launch_backend.c | 6 +-
src/backend/storage/ipc/dsm_registry.c | 12 +-
src/backend/storage/lmgr/lwlock.c | 206 +++++++++---------
src/include/storage/lwlock.h | 24 +-
src/test/modules/test_dsa/test_dsa.c | 6 +-
.../test_dsm_registry/test_dsm_registry.c | 3 +-
.../modules/test_radixtree/test_radixtree.c | 9 +-
src/test/modules/test_slru/test_slru.c | 6 +-
.../modules/test_tidstore/test_tidstore.c | 3 +-
11 files changed, 120 insertions(+), 173 deletions(-)
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index c01b9c7e6a4..880e897796a 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -864,7 +864,7 @@ apw_init_state(void *ptr)
{
AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr;
- LWLockInitialize(&state->lock, LWLockNewTrancheId());
+ LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm"));
state->bgworker_pid = InvalidPid;
state->pid_using_dumpfile = InvalidPid;
}
@@ -883,7 +883,6 @@ apw_init_shmem(void)
sizeof(AutoPrewarmSharedState),
apw_init_state,
&found);
- LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm");
return found;
}
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index f116d0648e5..da21ef56891 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name)
<literal>shmem_request_hook</literal>. To do so, first allocate a
<literal>tranche_id</literal> by calling:
<programlisting>
-int LWLockNewTrancheId(void)
+int LWLockNewTrancheId(const char *name)
</programlisting>
Next, initialize each LWLock, passing the new
<literal>tranche_id</literal> as an argument:
@@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id)
</para>
<para>
- Finally, each backend using the <literal>tranche_id</literal> should
- associate it with a <literal>tranche_name</literal> by calling:
-<programlisting>
-void LWLockRegisterTranche(int tranche_id, const char *tranche_name)
-</programlisting>
- </para>
-
- <para>
- A complete usage example of <function>LWLockNewTrancheId</function>,
- <function>LWLockInitialize</function>, and
- <function>LWLockRegisterTranche</function> can be found in
+ A complete usage example of <function>LWLockNewTrancheId</function> and
+ <function>LWLockInitialize</function> can be found in
<filename>contrib/pg_prewarm/autoprewarm.c</filename> in the
<productname>PostgreSQL</productname> source tree.
</para>
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index cd9547b03a3..de1b06df10d 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -101,7 +101,7 @@ typedef struct
struct InjectionPointsCtl *ActiveInjectionPoints;
#endif
int NamedLWLockTrancheRequests;
- NamedLWLockTranche *NamedLWLockTrancheArray;
+ char **NamedLWLockTrancheNames;
int *LWLockCounter;
LWLockPadded *MainLWLockArray;
slock_t *ProcStructLock;
@@ -761,7 +761,7 @@ save_backend_variables(BackendParameters *param,
#endif
param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests;
- param->NamedLWLockTrancheArray = NamedLWLockTrancheArray;
+ param->NamedLWLockTrancheNames = NamedLWLockTrancheNames;
param->LWLockCounter = LWLockCounter;
param->MainLWLockArray = MainLWLockArray;
param->ProcStructLock = ProcStructLock;
@@ -1022,7 +1022,7 @@ restore_backend_variables(BackendParameters *param)
#endif
NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests;
- NamedLWLockTrancheArray = param->NamedLWLockTrancheArray;
+ NamedLWLockTrancheNames = param->NamedLWLockTrancheNames;
LWLockCounter = param->LWLockCounter;
MainLWLockArray = param->MainLWLockArray;
ProcStructLock = param->ProcStructLock;
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index ca12815f4a8..97130925106 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found)
entry->type = DSMR_ENTRY_TYPE_DSA;
/* Initialize the LWLock tranche for the DSA. */
- state->tranche = LWLockNewTrancheId();
- LWLockRegisterTranche(state->tranche, name);
+ state->tranche = LWLockNewTrancheId(name);
/* Initialize the DSA. */
ret = dsa_create(state->tranche);
@@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found)
ereport(ERROR,
(errmsg("requested DSA already attached to current process")));
- /* Initialize existing LWLock tranche for the DSA. */
- LWLockRegisterTranche(state->tranche, name);
-
/* Attach to existing DSA. */
ret = dsa_attach(state->handle);
dsa_pin_mapping(ret);
@@ -378,8 +374,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
entry->type = DSMR_ENTRY_TYPE_DSH;
/* Initialize the LWLock tranche for the hash table. */
- dsh_state->tranche = LWLockNewTrancheId();
- LWLockRegisterTranche(dsh_state->tranche, name);
+ dsh_state->tranche = LWLockNewTrancheId(name);
/* Initialize the DSA for the hash table. */
dsa = dsa_create(dsh_state->tranche);
@@ -409,9 +404,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found)
ereport(ERROR,
(errmsg("requested DSHash already attached to current process")));
- /* Initialize existing LWLock tranche for the hash table. */
- LWLockRegisterTranche(dsh_state->tranche, name);
-
/* Attach to existing DSA for the hash table. */
dsa = dsa_attach(dsh_state->dsa_handle);
dsa_pin_mapping(dsa);
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index a4aecd1fbc3..b4636d86ca6 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0,
* in lwlocklist.h. We absorb the names of these tranches, too.
*
* 3. Extensions can create new tranches, via either RequestNamedLWLockTranche
- * or LWLockRegisterTranche. The names of these that are known in the current
- * process appear in LWLockTrancheNames[].
+ * or LWLockNewTrancheId. These names are stored in shared memory and can be
+ * accessed via NamedLWLockTrancheNames.
*
* All these names are user-visible as wait event names, so choose with care
* ... and do not forget to update the documentation's list of wait events.
@@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) ==
/*
* This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and
- * stores the names of all dynamically-created tranches known to the current
- * process. Any unused entries in the array will contain NULL.
+ * points to the shared memory locations of the names of all
+ * dynamically-created tranches. Backends inherit the pointer by fork from the
+ * postmaster (except in the EXEC_BACKEND case, where we have special measures
+ * to pass it down).
*/
-static const char **LWLockTrancheNames = NULL;
-static int LWLockTrancheNamesAllocated = 0;
+char **NamedLWLockTrancheNames = NULL;
/*
* This points to the main array of LWLocks in shared memory. Backends inherit
@@ -184,20 +185,22 @@ typedef struct NamedLWLockTrancheRequest
} NamedLWLockTrancheRequest;
static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL;
-static int NamedLWLockTrancheRequestsAllocated = 0;
/*
- * NamedLWLockTrancheRequests is both the valid length of the request array,
- * and the length of the shared-memory NamedLWLockTrancheArray later on.
- * This variable and NamedLWLockTrancheArray are non-static so that
- * postmaster.c can copy them to child processes in EXEC_BACKEND builds.
+ * NamedLWLockTrancheRequests is the valid length of the request array. This
+ * variable is non-static so that postmaster.c can copy them to child processes
+ * in EXEC_BACKEND builds.
*/
int NamedLWLockTrancheRequests = 0;
/* points to data in shared memory: */
-NamedLWLockTranche *NamedLWLockTrancheArray = NULL;
int *LWLockCounter = NULL;
+/* backend-local counter of registered tranches */
+static int LocalLWLockCounter;
+
+#define MAX_NAMED_TRANCHES 256
+
static void InitializeLWLocks(void);
static inline void LWLockReportWaitStart(LWLock *lock);
static inline void LWLockReportWaitEnd(void);
@@ -392,31 +395,28 @@ Size
LWLockShmemSize(void)
{
Size size;
- int i;
int numLocks = NUM_FIXED_LWLOCKS;
/* Calculate total number of locks needed in the main array. */
numLocks += NumLWLocksForNamedTranches();
- /* Space for dynamic allocation counter, plus room for alignment. */
- size = sizeof(int) + LWLOCK_PADDED_SIZE;
+ /* Space for dynamic allocation counter. */
+ size = MAXALIGN(sizeof(int));
- /* Space for the LWLock array. */
- size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded)));
+ /* Space for named tranches. */
+ size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *)));
+ size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN));
- /* space for named tranches. */
- size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche)));
-
- /* space for name of each tranche. */
- for (i = 0; i < NamedLWLockTrancheRequests; i++)
- size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1);
+ /* Space for the LWLock array, plus room for cache line alignment. */
+ size = add_size(size, LWLOCK_PADDED_SIZE);
+ size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded)));
return size;
}
/*
* Allocate shmem space for the main LWLock array and all tranches and
- * initialize it. We also register extension LWLock tranches here.
+ * initialize it.
*/
void
CreateLWLocks(void)
@@ -432,7 +432,16 @@ CreateLWLocks(void)
/* Initialize the dynamic-allocation counter for tranches */
LWLockCounter = (int *) ptr;
*LWLockCounter = LWTRANCHE_FIRST_USER_DEFINED;
- ptr += sizeof(int);
+ ptr += MAXALIGN(sizeof(int));
+
+ /* Initialize tranche names */
+ NamedLWLockTrancheNames = (char **) ptr;
+ ptr += MAX_NAMED_TRANCHES * sizeof(char *);
+ for (int i = 0; i < MAX_NAMED_TRANCHES; i++)
+ {
+ NamedLWLockTrancheNames[i] = ptr;
+ ptr += NAMEDATALEN;
+ }
/* Ensure desired alignment of LWLock array */
ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE;
@@ -441,11 +450,6 @@ CreateLWLocks(void)
/* Initialize all LWLocks */
InitializeLWLocks();
}
-
- /* Register named extension LWLock tranches in the current process. */
- for (int i = 0; i < NamedLWLockTrancheRequests; i++)
- LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId,
- NamedLWLockTrancheArray[i].trancheName);
}
/*
@@ -454,7 +458,6 @@ CreateLWLocks(void)
static void
InitializeLWLocks(void)
{
- int numNamedLocks = NumLWLocksForNamedTranches();
int id;
int i;
int j;
@@ -485,32 +488,18 @@ InitializeLWLocks(void)
*/
if (NamedLWLockTrancheRequests > 0)
{
- char *trancheNames;
-
- NamedLWLockTrancheArray = (NamedLWLockTranche *)
- &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks];
-
- trancheNames = (char *) NamedLWLockTrancheArray +
- (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche));
lock = &MainLWLockArray[NUM_FIXED_LWLOCKS];
for (i = 0; i < NamedLWLockTrancheRequests; i++)
{
NamedLWLockTrancheRequest *request;
- NamedLWLockTranche *tranche;
- char *name;
+ int tranche;
request = &NamedLWLockTrancheRequestArray[i];
- tranche = &NamedLWLockTrancheArray[i];
-
- name = trancheNames;
- trancheNames += strlen(request->tranche_name) + 1;
- strcpy(name, request->tranche_name);
- tranche->trancheId = LWLockNewTrancheId();
- tranche->trancheName = name;
+ tranche = LWLockNewTrancheId(request->tranche_name);
for (j = 0; j < request->num_lwlocks; j++, lock++)
- LWLockInitialize(&lock->lock, tranche->trancheId);
+ LWLockInitialize(&lock->lock, tranche);
}
}
}
@@ -562,59 +551,44 @@ GetNamedLWLockTranche(const char *tranche_name)
}
/*
- * Allocate a new tranche ID.
+ * Allocate a new tranche ID with the provided name.
*/
int
-LWLockNewTrancheId(void)
+LWLockNewTrancheId(const char *name)
{
int result;
- /* We use the ShmemLock spinlock to protect LWLockCounter */
- SpinLockAcquire(ShmemLock);
- result = (*LWLockCounter)++;
- SpinLockRelease(ShmemLock);
+ if (!name)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("tranche name cannot be NULL")));
- return result;
-}
+ if (strlen(name) >= NAMEDATALEN)
+ ereport(ERROR,
+ (errcode(ERRCODE_NAME_TOO_LONG),
+ errmsg("tranche name too long"),
+ errdetail("LWLock tranche names must be no longer than %d bytes.",
+ NAMEDATALEN - 1)));
-/*
- * Register a dynamic tranche name in the lookup table of the current process.
- *
- * This routine will save a pointer to the tranche name passed as an argument,
- * so the name should be allocated in a backend-lifetime context
- * (shared memory, TopMemoryContext, static constant, or similar).
- *
- * The tranche name will be user-visible as a wait event name, so try to
- * use a name that fits the style for those.
- */
-void
-LWLockRegisterTranche(int tranche_id, const char *tranche_name)
-{
- /* This should only be called for user-defined tranches. */
- if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED)
- return;
-
- /* Convert to array index. */
- tranche_id -= LWTRANCHE_FIRST_USER_DEFINED;
+ /* We use the ShmemLock spinlock to protect LWLockCounter */
+ SpinLockAcquire(ShmemLock);
- /* If necessary, create or enlarge array. */
- if (tranche_id >= LWLockTrancheNamesAllocated)
+ if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES)
{
- int newalloc;
+ SpinLockRelease(ShmemLock);
+ ereport(ERROR,
+ (errmsg("maximum number of tranches already registered"),
+ errdetail("No more than %d tranches may be registered.",
+ MAX_NAMED_TRANCHES)));
+ }
- newalloc = pg_nextpower2_32(Max(8, tranche_id + 1));
+ result = (*LWLockCounter)++;
+ LocalLWLockCounter = *LWLockCounter;
+ strlcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name, NAMEDATALEN);
- if (LWLockTrancheNames == NULL)
- LWLockTrancheNames = (const char **)
- MemoryContextAllocZero(TopMemoryContext,
- newalloc * sizeof(char *));
- else
- LWLockTrancheNames =
- repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc);
- LWLockTrancheNamesAllocated = newalloc;
- }
+ SpinLockRelease(ShmemLock);
- LWLockTrancheNames[tranche_id] = tranche_name;
+ return result;
}
/*
@@ -637,27 +611,33 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
if (!process_shmem_requests_in_progress)
elog(FATAL, "cannot request additional LWLocks outside shmem_request_hook");
+ if (!tranche_name)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_NAME),
+ errmsg("tranche name cannot be NULL")));
+
+ if (strlen(tranche_name) >= NAMEDATALEN)
+ ereport(ERROR,
+ (errcode(ERRCODE_NAME_TOO_LONG),
+ errmsg("tranche name too long"),
+ errdetail("LWLock tranche names must be no longer than %d bytes.",
+ NAMEDATALEN - 1)));
+
if (NamedLWLockTrancheRequestArray == NULL)
{
- NamedLWLockTrancheRequestsAllocated = 16;
NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *)
MemoryContextAlloc(TopMemoryContext,
- NamedLWLockTrancheRequestsAllocated
+ MAX_NAMED_TRANCHES
* sizeof(NamedLWLockTrancheRequest));
}
- if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated)
- {
- int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1);
-
- NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *)
- repalloc(NamedLWLockTrancheRequestArray,
- i * sizeof(NamedLWLockTrancheRequest));
- NamedLWLockTrancheRequestsAllocated = i;
- }
+ if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES)
+ ereport(ERROR,
+ (errmsg("maximum number of tranches already registered"),
+ errdetail("No more than %d tranches may be registered.",
+ MAX_NAMED_TRANCHES)));
request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests];
- Assert(strlen(tranche_name) + 1 <= NAMEDATALEN);
strlcpy(request->tranche_name, tranche_name, NAMEDATALEN);
request->num_lwlocks = num_lwlocks;
NamedLWLockTrancheRequests++;
@@ -669,6 +649,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
void
LWLockInitialize(LWLock *lock, int tranche_id)
{
+ /* verify the tranche_id is valid */
+ (void) GetLWTrancheName(tranche_id);
+
pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK);
#ifdef LOCK_DEBUG
pg_atomic_init_u32(&lock->nwaiters, 0);
@@ -709,18 +692,23 @@ GetLWTrancheName(uint16 trancheId)
if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
return BuiltinTrancheNames[trancheId];
+ /* verify the trancheId is valid */
+ if (trancheId >= LocalLWLockCounter)
+ {
+ SpinLockAcquire(ShmemLock);
+ LocalLWLockCounter = *LWLockCounter;
+ SpinLockRelease(ShmemLock);
+
+ if (trancheId >= LocalLWLockCounter)
+ elog(ERROR, "tranche %d is not registered", trancheId);
+ }
+
/*
- * It's an extension tranche, so look in LWLockTrancheNames[]. However,
- * it's possible that the tranche has never been registered in the current
- * process, in which case give up and return "extension".
+ * It's an extension tranche, so look in NamedLWLockTrancheNames.
*/
trancheId -= LWTRANCHE_FIRST_USER_DEFINED;
- if (trancheId >= LWLockTrancheNamesAllocated ||
- LWLockTrancheNames[trancheId] == NULL)
- return "extension";
-
- return LWLockTrancheNames[trancheId];
+ return NamedLWLockTrancheNames[trancheId];
}
/*
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index f9cf57f8d26..3877aaa7f03 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -73,14 +73,7 @@ typedef union LWLockPadded
extern PGDLLIMPORT LWLockPadded *MainLWLockArray;
-/* struct for storing named tranche information */
-typedef struct NamedLWLockTranche
-{
- int trancheId;
- char *trancheName;
-} NamedLWLockTranche;
-
-extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray;
+extern PGDLLIMPORT char **NamedLWLockTrancheNames;
extern PGDLLIMPORT int NamedLWLockTrancheRequests;
extern PGDLLIMPORT int *LWLockCounter;
@@ -158,18 +151,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name);
/*
* There is another, more flexible method of obtaining lwlocks. First, call
- * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from
- * a shared counter. Next, each individual process using the tranche should
- * call LWLockRegisterTranche() to associate that tranche ID with a name.
- * Finally, LWLockInitialize should be called just once per lwlock, passing
- * the tranche ID as an argument.
- *
- * It may seem strange that each process using the tranche must register it
- * separately, but dynamic shared memory segments aren't guaranteed to be
- * mapped at the same address in all coordinating backends, so storing the
- * registration in the main shared memory segment wouldn't work for that case.
+ * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared
+ * counter. Second, LWLockInitialize should be called just once per lwlock,
+ * passing the tranche ID as an argument.
*/
-extern int LWLockNewTrancheId(void);
+extern int LWLockNewTrancheId(const char *name);
extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name);
extern void LWLockInitialize(LWLock *lock, int tranche_id);
diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c
index cd24d0f4873..01d5c6fa67f 100644
--- a/src/test/modules/test_dsa/test_dsa.c
+++ b/src/test/modules/test_dsa/test_dsa.c
@@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS)
dsa_pointer p[100];
/* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(tranche_id, "test_dsa");
+ tranche_id = LWLockNewTrancheId("test_dsa");
a = dsa_create(tranche_id);
for (int i = 0; i < 100; i++)
@@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS)
ResourceOwner childowner;
/* XXX: this tranche is leaked */
- tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(tranche_id, "test_dsa");
+ tranche_id = LWLockNewTrancheId("test_dsa");
/* Create DSA in parent resource owner */
a = dsa_create(tranche_id);
diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c
index 141c8ed1b34..4cc2ccdac3f 100644
--- a/src/test/modules/test_dsm_registry/test_dsm_registry.c
+++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c
@@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr)
{
TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr;
- LWLockInitialize(&dsm->lck, LWLockNewTrancheId());
+ LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry"));
dsm->val = 0;
}
@@ -61,7 +61,6 @@ tdr_attach_shmem(void)
sizeof(TestDSMRegistryStruct),
init_tdr_dsm,
&found);
- LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry");
if (tdr_dsa == NULL)
tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found);
diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c
index 80ad0296164..787162c8793 100644
--- a/src/test/modules/test_radixtree/test_radixtree.c
+++ b/src/test/modules/test_radixtree/test_radixtree.c
@@ -124,10 +124,9 @@ test_empty(void)
rt_iter *iter;
uint64 key;
#ifdef TEST_SHARED_RT
- int tranche_id = LWLockNewTrancheId();
+ int tranche_id = LWLockNewTrancheId("test_radix_tree");
dsa_area *dsa;
- LWLockRegisterTranche(tranche_id, "test_radix_tree");
dsa = dsa_create(tranche_id);
radixtree = rt_create(dsa, tranche_id);
#else
@@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc)
uint64 *keys;
int children = test_info->nkeys;
#ifdef TEST_SHARED_RT
- int tranche_id = LWLockNewTrancheId();
+ int tranche_id = LWLockNewTrancheId("test_radix_tree");
dsa_area *dsa;
- LWLockRegisterTranche(tranche_id, "test_radix_tree");
dsa = dsa_create(tranche_id);
radixtree = rt_create(dsa, tranche_id);
#else
@@ -304,10 +302,9 @@ test_random(void)
int num_keys = 100000;
uint64 *keys;
#ifdef TEST_SHARED_RT
- int tranche_id = LWLockNewTrancheId();
+ int tranche_id = LWLockNewTrancheId("test_radix_tree");
dsa_area *dsa;
- LWLockRegisterTranche(tranche_id, "test_radix_tree");
dsa = dsa_create(tranche_id);
radixtree = rt_create(dsa, tranche_id);
#else
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 32750930e43..8c0367eeee4 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -232,11 +232,9 @@ test_slru_shmem_startup(void)
(void) MakePGDirectory(slru_dir_name);
/* initialize the SLRU facility */
- test_tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(test_tranche_id, "test_slru_tranche");
+ test_tranche_id = LWLockNewTrancheId("test_slru_tranche");
- test_buffer_tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche");
+ test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche");
TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
SimpleLruInit(TestSlruCtl, "TestSLRU",
diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c
index eb16e0fbfa6..0c8f43867e5 100644
--- a/src/test/modules/test_tidstore/test_tidstore.c
+++ b/src/test/modules/test_tidstore/test_tidstore.c
@@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS)
{
int tranche_id;
- tranche_id = LWLockNewTrancheId();
- LWLockRegisterTranche(tranche_id, "test_tidstore");
+ tranche_id = LWLockNewTrancheId("test_tidstore");
tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id);
--
2.39.5 (Apple Git-154)
v20-0002-Tests-for-LWLock-tranche-registration-improvemen.patch (text/plain, 14.7K) download
From 65c80071e74c36328873dfa9cf6ce8f537d83850 Mon Sep 17 00:00:00 2001
From: Sami Imseih <simseih@amazon.com>
Date: Fri, 22 Aug 2025 00:35:45 -0500
Subject: [PATCH v20 2/2] Tests for LWLock tranche registration improvements
---
src/test/modules/Makefile | 3 +-
src/test/modules/meson.build | 1 +
src/test/modules/test_tranches/.gitignore | 4 +
src/test/modules/test_tranches/Makefile | 23 +++
src/test/modules/test_tranches/meson.build | 33 +++
.../test_tranches/t/001_test_tranches.pl | 120 +++++++++++
.../test_tranches/test_tranches--1.0.sql | 35 ++++
.../modules/test_tranches/test_tranches.c | 191 ++++++++++++++++++
.../test_tranches/test_tranches.control | 6 +
9 files changed, 415 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/test_tranches/.gitignore
create mode 100644 src/test/modules/test_tranches/Makefile
create mode 100644 src/test/modules/test_tranches/meson.build
create mode 100644 src/test/modules/test_tranches/t/001_test_tranches.pl
create mode 100644 src/test/modules/test_tranches/test_tranches--1.0.sql
create mode 100644 src/test/modules/test_tranches/test_tranches.c
create mode 100644 src/test/modules/test_tranches/test_tranches.control
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 903a8ac151a..fd7aa4675c5 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -44,7 +44,8 @@ SUBDIRS = \
test_tidstore \
unsafe_tests \
worker_spi \
- xid_wraparound
+ xid_wraparound \
+ test_tranches
ifeq ($(enable_injection_points),yes)
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 93be0f57289..52883dfb496 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -45,3 +45,4 @@ subdir('typcache')
subdir('unsafe_tests')
subdir('worker_spi')
subdir('xid_wraparound')
+subdir('test_tranches')
diff --git a/src/test/modules/test_tranches/.gitignore b/src/test/modules/test_tranches/.gitignore
new file mode 100644
index 00000000000..5dcb3ff9723
--- /dev/null
+++ b/src/test/modules/test_tranches/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_tranches/Makefile b/src/test/modules/test_tranches/Makefile
new file mode 100644
index 00000000000..5c9e78084da
--- /dev/null
+++ b/src/test/modules/test_tranches/Makefile
@@ -0,0 +1,23 @@
+# src/test/modules/test_tranches/Makefile
+
+MODULE_big = test_tranches
+OBJS = \
+ $(WIN32RES) \
+ test_tranches.o
+PGFILEDESC = "test_tranches - test code LWLock tranche management"
+
+EXTENSION = test_tranches
+DATA = test_tranches--1.0.sql
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_tranches
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_tranches/meson.build b/src/test/modules/test_tranches/meson.build
new file mode 100644
index 00000000000..976ce9f0a25
--- /dev/null
+++ b/src/test/modules/test_tranches/meson.build
@@ -0,0 +1,33 @@
+# Copyright (c) 2024-2025, PostgreSQL Global Development Group
+
+test_tranches_sources = files(
+ 'test_tranches.c',
+)
+
+if host_system == 'windows'
+ test_tranches_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+ '--NAME', 'test_tranches',
+ '--FILEDESC', 'test_tranches - test code LWLock tranche management',])
+endif
+
+test_tranches = shared_module('test_tranches',
+ test_tranches_sources,
+ kwargs: pg_test_mod_args,
+)
+test_install_libs += test_tranches
+
+test_install_data += files(
+ 'test_tranches.control',
+ 'test_tranches--1.0.sql',
+)
+
+tests += {
+ 'name': 'test_tranches',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_test_tranches.pl',
+ ],
+ },
+}
\ No newline at end of file
diff --git a/src/test/modules/test_tranches/t/001_test_tranches.pl b/src/test/modules/test_tranches/t/001_test_tranches.pl
new file mode 100644
index 00000000000..477035bdb99
--- /dev/null
+++ b/src/test/modules/test_tranches/t/001_test_tranches.pl
@@ -0,0 +1,120 @@
+use strict;
+use warnings FATAL => 'all';
+
+use List::Util qw(min);
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+#
+# Create the cluster without any requested tranches
+#
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init();
+$node->append_conf('postgresql.conf',
+ qq(shared_preload_libraries='test_tranches'));
+$node->append_conf('postgresql.conf',
+ qq(test_tranches.requested_named_tranches=0));
+$node->start();
+$node->safe_psql('postgres', q(CREATE EXTENSION test_tranches));
+
+my $first_user_defined = $node->safe_psql('postgres', q(select test_tranches_get_first_user_defined()));
+my @user_defined = ($first_user_defined .. $first_user_defined + 5);
+my ($second_user_defined,
+ $third_user_defined,
+ $fourth_user_defined,
+ $fifth_user_defined,
+ $sixth_user_defined) = @user_defined[1..5];
+#
+# Run lookup tests to ensure the correct tranche names are returned
+# and an error is raised for unregistered tranches
+#
+$node->safe_psql('postgres', "select test_tranches_new(5);");
+my ($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{
+ select test_tranches_lookup(1);
+ select test_tranches_lookup($first_user_defined);
+ select test_tranches_lookup($second_user_defined);
+ select test_tranches_lookup($third_user_defined);
+ select test_tranches_lookup($fourth_user_defined);
+ select test_tranches_lookup($fifth_user_defined);
+ select test_tranches_lookup($sixth_user_defined);
+ },
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche 100 is not registered/, "unregistered tranche error");
+like("$stdout",
+ qr/ShmemIndex.*test_lock__0.*test_lock__1.*test_lock__2.*test_lock__3.*test_lock__4/s,
+ "match tranche names without requested tranches");
+
+#
+# Test the error for long tranche names
+#
+my $good_tranche_name = 'A' x 63;
+my $bad_tranche_name = 'B' x 64; # MAX_NAMED_TRANCHES_NAME_LEN
+$node->safe_psql('postgres', qq{select test_tranches_new_tranche('$good_tranche_name');});
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{
+ select test_tranches_new_tranche('$bad_tranche_name');
+ },
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche name too long/, "tranche name too long");
+
+$node->restart();
+
+#
+# Test the error when > MAX_NAMED_TRANCHES named tranches registered.
+#
+$node->safe_psql('postgres', qq{select test_tranches_new(255)});
+$node->safe_psql('postgres', qq{select test_tranches_new(1)});
+($result, $stdout, $stderr) = $node->psql('postgres', qq{select test_tranches_new(1);}, on_error_stop => 0);
+like("$stderr", qr/ERROR: maximum number of tranches already registered/, "too many tranches registered");
+
+#
+# Repeat the lookup test with 2 requested tranches
+#
+$node->append_conf('postgresql.conf',
+ qq(test_tranches.requested_named_tranches=2));
+$node->restart();
+
+$first_user_defined = $first_user_defined;
+@user_defined = ($first_user_defined .. $first_user_defined + 5);
+($second_user_defined,
+ $third_user_defined,
+ $fourth_user_defined,
+ $fifth_user_defined,
+ $sixth_user_defined) = @user_defined[1..5];
+
+$node->safe_psql('postgres', "select test_tranches_new(3);");
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{
+ select test_tranches_lookup(1);
+ select test_tranches_lookup($first_user_defined);
+ select test_tranches_lookup($second_user_defined);
+ select test_tranches_lookup($third_user_defined);
+ select test_tranches_lookup($fourth_user_defined);
+ select test_tranches_lookup($fifth_user_defined);
+ select test_tranches_lookup($sixth_user_defined);
+ },
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche 100 is not registered/, "unregistered tranche error");
+like("$stdout", qr/ShmemIndex.*test_lock_0.*test_lock_1.*test_lock__2.*test_lock__3.*test_lock__4/s,
+ "match tranche names with requested tranches");
+
+#
+# Test lwlock initialize
+#
+$node->safe_psql('postgres', qq{select test_tranches_lwlock_initialize($first_user_defined)});
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{select test_tranches_lwlock_initialize($sixth_user_defined)},
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche 100 is not registered/, "LWLock intialization error on invalid tranche name");
+
+#
+# Test error for NULL tranche name
+#
+($result, $stdout, $stderr) = $node->psql('postgres',
+ qq{select test_tranches_new_tranche(NULL)},
+ on_error_stop => 0);
+like("$stderr", qr/ERROR: tranche name cannot be NULL/, "NULL tranche name");
+
+done_testing();
diff --git a/src/test/modules/test_tranches/test_tranches--1.0.sql b/src/test/modules/test_tranches/test_tranches--1.0.sql
new file mode 100644
index 00000000000..f504098e04a
--- /dev/null
+++ b/src/test/modules/test_tranches/test_tranches--1.0.sql
@@ -0,0 +1,35 @@
+-- test_tranches--1.0.sql
+
+CREATE FUNCTION test_tranches_new(bigint)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_tranches_new'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_lookup(int)
+RETURNS text
+AS 'MODULE_PATHNAME', 'test_tranches_lookup'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_get_named_lwlock(text, int)
+RETURNS int
+AS 'MODULE_PATHNAME', 'test_tranches_get_named_lwlock'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_get_first_user_defined()
+RETURNS int
+AS 'MODULE_PATHNAME', 'test_tranches_get_first_user_defined'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION test_tranches_lwlock_initialize(int)
+RETURNS void
+AS 'MODULE_PATHNAME', 'test_tranches_lwlock_initialize'
+LANGUAGE C STRICT;
+
+/*
+ * Function is CALLED ON NULL INPUT to allow NULL input
+ * for testing
+ */
+CREATE FUNCTION test_tranches_new_tranche(text)
+RETURNS int
+AS 'MODULE_PATHNAME', 'test_tranches_new_tranche'
+LANGUAGE C;
\ No newline at end of file
diff --git a/src/test/modules/test_tranches/test_tranches.c b/src/test/modules/test_tranches/test_tranches.c
new file mode 100644
index 00000000000..7d3031ace9f
--- /dev/null
+++ b/src/test/modules/test_tranches/test_tranches.c
@@ -0,0 +1,191 @@
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "storage/dsm_registry.h"
+#include "storage/ipc.h"
+#include "storage/lwlock.h"
+#include "storage/shmem.h"
+#include "utils/builtins.h"
+#include "utils/guc.h"
+#include "utils/injection_point.h"
+#include "utils/wait_classes.h"
+
+PG_MODULE_MAGIC;
+
+/* hooks */
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
+static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
+static void test_tranches_shmem_request(void);
+static void test_tranches_shmem_startup(void);
+
+/* GUC */
+static int test_tranches_requested_named_tranches = 0;
+
+typedef struct testTranchesSharedState
+{
+ int next_index;
+} testTranchesSharedState;
+
+static testTranchesSharedState * test_lwlock_ss = NULL;
+
+/*
+ * LWLock wait event masks. Copied from src/backend/utils/activity/wait_event.c
+ */
+#define WAIT_EVENT_CLASS_MASK 0xFF000000
+
+/*
+ * Module load callback
+ */
+void
+_PG_init(void)
+{
+ prev_shmem_request_hook = shmem_request_hook;
+ shmem_request_hook = test_tranches_shmem_request;
+ prev_shmem_startup_hook = shmem_startup_hook;
+ shmem_startup_hook = test_tranches_shmem_startup;
+
+ DefineCustomIntVariable("test_tranches.requested_named_tranches",
+ "Sets the number of locks created during shmem request",
+ NULL,
+ &test_tranches_requested_named_tranches,
+ 2,
+ 0,
+ UINT16_MAX,
+ PGC_POSTMASTER,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
+static void
+test_tranches_shmem_startup(void)
+{
+ bool found;
+
+ if (prev_shmem_startup_hook)
+ prev_shmem_startup_hook();
+
+ test_lwlock_ss = NULL;
+
+ test_lwlock_ss = ShmemInitStruct("test_tranches",
+ sizeof(testTranchesSharedState),
+ &found);
+ if (!found)
+ {
+ test_lwlock_ss->next_index = test_tranches_requested_named_tranches;
+ }
+}
+
+static Size
+test_tranches_memsize(void)
+{
+ Size size;
+
+ size = MAXALIGN(sizeof(testTranchesSharedState));
+
+ return size;
+}
+
+static void
+test_tranches_shmem_request(void)
+{
+ int i = 0;
+
+ if (prev_shmem_request_hook)
+ prev_shmem_request_hook();
+
+ RequestAddinShmemSpace(test_tranches_memsize());
+
+ for (i = 0; i < test_tranches_requested_named_tranches; i++)
+ {
+ char name[15];
+
+ snprintf(name, sizeof(name), "test_lock_%d", i);
+ RequestNamedLWLockTranche(name, i);
+ }
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_new);
+Datum
+test_tranches_new(PG_FUNCTION_ARGS)
+{
+ int64 num = PG_GETARG_INT64(0);
+ int i;
+
+ for (i = test_lwlock_ss->next_index; i < num + test_lwlock_ss->next_index; i++)
+ {
+ char name[50];
+
+ snprintf(name, 50, "test_lock__%d", i);
+
+ LWLockNewTrancheId(name);
+ }
+
+ test_lwlock_ss->next_index = i;
+
+ PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_new_tranche);
+Datum
+test_tranches_new_tranche(PG_FUNCTION_ARGS)
+{
+ char *tranche_name = NULL;
+
+ if (!PG_ARGISNULL(0))
+ tranche_name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+ PG_RETURN_INT32(LWLockNewTrancheId(tranche_name));
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_lookup);
+Datum
+test_tranches_lookup(PG_FUNCTION_ARGS)
+{
+ const char *tranche_name = GetLWLockIdentifier(PG_WAIT_LWLOCK & WAIT_EVENT_CLASS_MASK, PG_GETARG_INT32(0));
+
+ if (tranche_name)
+ PG_RETURN_TEXT_P(cstring_to_text(tranche_name));
+ else
+ PG_RETURN_NULL();
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_get_named_lwlock);
+Datum
+test_tranches_get_named_lwlock(PG_FUNCTION_ARGS)
+{
+ int i;
+ LWLockPadded *locks;
+
+ locks = GetNamedLWLockTranche(text_to_cstring(PG_GETARG_TEXT_PP(0)));
+
+ for (i = 0; i < PG_GETARG_INT32(1); i++)
+ {
+ LWLock *lock = &locks[i].lock;
+
+ LWLockAcquire(lock, LW_SHARED);
+ LWLockRelease(lock);
+ }
+
+ PG_RETURN_INT32(i);
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_get_first_user_defined);
+Datum
+test_tranches_get_first_user_defined(PG_FUNCTION_ARGS)
+{
+ return LWTRANCHE_FIRST_USER_DEFINED;
+}
+
+PG_FUNCTION_INFO_V1(test_tranches_lwlock_initialize);
+Datum
+test_tranches_lwlock_initialize(PG_FUNCTION_ARGS)
+{
+ LWLock lock;
+
+ LWLockInitialize(&lock, PG_GETARG_INT32(0));
+
+ PG_RETURN_VOID();
+}
diff --git a/src/test/modules/test_tranches/test_tranches.control b/src/test/modules/test_tranches/test_tranches.control
new file mode 100644
index 00000000000..8e6254a7e43
--- /dev/null
+++ b/src/test/modules/test_tranches/test_tranches.control
@@ -0,0 +1,6 @@
+# test_tranches.control
+
+comment = 'Test LWLock tranch names tracking'
+default_version = '1.0'
+relocatable = false
+module_pathname = '$libdir/test_tranches'
\ No newline at end of file
--
2.39.5 (Apple Git-154)
view thread (107+ messages) latest in thread
Message-ID: <aLMHVosexT5D-Ybz@nathan>
Permalink: ../aLMHVosexT5D-Ybz@nathan/
Also on: postgresql.org/message-id/aLMHVosexT5D-Ybz@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, bertranddrouvot.pg@gmail.com, tgl@sss.pgh.pa.us, andres@anarazel.de, rahilasyed90@gmail.com
Subject: Re: Improve LWLock tranche name visibility across backends
In-Reply-To: <aLMHVosexT5D-Ybz@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