Re: Clean up NamedLWLockTranche stuff

On Thu, Mar 26, 2026 at 02:16:52PM +0200, Heikki Linnakangas wrote:
> At postmaster startup, NamedLWLockTrancheRequests points to a
> backend-private array. But after startup, and always in backends, it points
> to a copy in shared memory and LocalNamedLWLockTrancheRequestArray is used
> to hold the original. It took me a while to realize that
> NamedLWLockTrancheRequests in shared memory is *not* updated when you call
> LWLockNewTrancheId(), it only holds the requests made with
> RequestNamedLWLockTranche() before startup.

Right. LocalNamedLWLockTrancheRequestArray is needed so that we can
re-initialize shared memory after a crash. See commit c3cc2ab87d.

> I propose the attached refactorings to make this less confusing. See commit
> messages for details.

Thanks for doing this, Heikki. I agree that we ought to make this stuff
cleaner. I've asked Sami Imseih, who worked on LWLocks with me last year,
to look at this patch set, too.

> Subject: [PATCH v1 1/5] Rename MAX_NAMED_TRANCHES to MAX_USER_DEFINED_TRANCHES

Seems fine to me.

0002:

> + foreach(lc, NamedLWLockTrancheRequests)

nitpick: These foreach loops seem like good opportunities to use
foreach_ptr.

The comment atop NumLWLocksForNamedTranches might benefit from mentioning
RequestNamedLWLockTranche() and the fact that it only works in the
postmaster. Perhaps an assertion is warranted, too.

+ SpinLockAcquire(ShmemLock);
+ LocalNumUserDefinedTranches = LWLockTranches->num_user_defined;
+ SpinLockRelease(ShmemLock);

Not critical, but it might be worth making num_user_defined an atomic.

Overall, 0002 looks reasonable to me upon a first read-through.

> Subject: [PATCH v1 3/5] Use a separate spinlock to protect LWLockTranches

Seems fine to me.

0004:

> +++ b/src/backend/storage/ipc/shmem.c
> @@ -379,7 +379,8 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
>
> Assert(ShmemIndex != NULL);
>
> - LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);
> + if (IsUnderPostmaster)
> + LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);

Am I understanding that we assume ShmemInitStruct() is only called by the
postmaster when there are no other backends yet?

0005:

> - if (IsUnderPostmaster)
> - LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);
> + LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);

Oh, this reverts many of these changes from 0004. Maybe the patches could
be reordered to avoid this?

--
nathan