Re: Improve LWLock tranche name visibility across backends
Sami Imseih <samimseih@gmail.com>
From: Sami Imseih <samimseih@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Rahila Syed <rahilasyed90@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-08-29T13:30:06Z
Lists: pgsql-hackers
Attachments
- v18-0002-Move-dynamically-allocated-tranche-names-to-shar.patch (application/octet-stream) patch v18-0002
- v18-0003-Fix-EXEC_BACKEND-segfault-on-NamedLWLockTrancheR.patch (application/octet-stream) patch v18-0003
- v18-0001-dsm_registry-Use-one-LWLock-tranche-for-dshash-t.patch (application/octet-stream) patch v18-0001
> On Thu, Aug 28, 2025 at 05:53:23PM -0500, Sami Imseih wrote:
> > Just a few things that were discussed earlier, that I incorporated now.
> >
> > 1/ We should be checking that tranche_name is NOT NULL when
> > LWLockNewTrancheId or RequestNamedLWLockTranche is called.
>
> Right, if not strlen() does segfault.
>
> In addition to checking for NULL, should we also check for empty string? Currently,
> the patch does accept strlen(tranche_name) == 0.
I am not inclined to prevent an empty string. It's currently allowed and rather
not change that.
> ```
> typedef struct NamedLWLockTranche
> {
> char trancheName[NAMEDATALEN];
> int num_lwlocks;
> } NamedLWLockTranche;
> ```
> if there is no interest to backpatch [0], maybe we should just make this
> change as part of this patch set. What do you think? I can make this change
> in v18.
Here is v18. It includes a third patch to fix the issue identified in
[0], which can
be applied to HEAD as part of this thread. If we want to backpatch the stable
branches, the version in [0] is suitable.
Note that I created a LWLockNewTrancheIdInternal which takes a tranch
name and number of lwlocks. The Internal version is used during startup when
requested lwlocks are appended to shared memory, and the existing
LWLockNewTrancheId calls the internal version with 0 lwlocks.
This keeps all the logic to appending a new tranche ( while holding
the spinlock )
in the same routine.
--
Sami
Commits
-
test_dsa: Avoid leaking LWLock tranches.
- c5c74282f2ea 19 (unreleased) landed
-
Teach DSM registry to ERROR if attaching to an uninitialized entry.
- b26d76f64327 18.2 landed
- ac2800ddc185 17.8 landed
- 1165a933aab1 19 (unreleased) landed
-
Add a test harness for the LWLock tranche code.
- 16607718c010 19 (unreleased) landed
-
Revert recent change to RequestNamedLWLockTranche().
- d814d7fc3d52 19 (unreleased) landed
-
Move dynamically-allocated LWLock tranche names to shared memory.
- 38b602b0289f 19 (unreleased) landed
-
Prepare DSM registry for upcoming changes to LWLock tranche names.
- 5487058b56e0 19 (unreleased) landed
-
Add GetNamedDSA() and GetNamedDSHash().
- fe07100e82b0 19 (unreleased) cited