Re: Improve LWLock tranche name visibility across backends

On Mon, Sep 01, 2025 at 06:35:55PM +0530, Rahila Syed wrote:
> Please see below for some comments regarding v20 patch.

Thanks for looking.

> 1. Following unused declaration is present in the lwlock.h file.
>
> extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name);

Whoops, good catch.

> If the intention is to delay acquiring the spin lock when trancheId is
> less than LocalLWLockCounter, it seems possible that we might read
> NamedLWLockTrancheNames in the GetLWTrancheName function after
> LocalLWLockCounter has been updated, but before NamedLWLockTrancheNames
> has been updated in the LWLockNewTrancheId function. To prevent reading
> an outdated tranche name, we should acquire the spin lock unconditionally
> in the GetLWTrancheName function

We only ever add new tranche names to the end of the list, so we should be
able to avoid the lock for the vast majority of tranche name lookups. We
hold the lock while we increment LWLockCounter and add a name, and also
whenever we update LocalLWLockCounter. This should prevent any unsafe
accesses to NamedLWLockTrancheNames.

I've added some additional commentary about this in v21.

> + ereport(ERROR,
> + (errmsg("maximum number of tranches already registered"),
> + errdetail("No more than %d tranches may be registered.",
> + MAX_NAMED_TRANCHES)));
> + }
>
> Since this patch sets a maximum limit on the number of LW lock tranches
> that can be registered, would it make sense to offer a configurable
> option rather than using a hard-coded MAX_NUM_TRANCHES? This will allow
> users who have reached the maximum limit to register their LW Locks.

Hm. I'm not sure. I think it would be good to offer a way to accommodate
more tranche names that didn't involve recompiling, but I don't know
whether that situation is likely enough in practice to add yet another GUC.
Thus far, we've been operating under the assumption that we'll be able to
choose a number far beyond any realistic usage. If others have opinions
about this, please share...

--
nathan