public inbox for [email protected]
help / color / mirror / Atom feedRe: Clean up NamedLWLockTranche stuff
14+ messages / 4 participants
[nested] [flat]
* Re: Clean up NamedLWLockTranche stuff
@ 2026-03-26 14:37 Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Nathan Bossart @ 2026-03-26 14:37 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
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
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
@ 2026-03-26 15:21 ` Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Heikki Linnakangas @ 2026-03-26 15:21 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
On 26/03/2026 16:37, Nathan Bossart wrote:
> On Thu, Mar 26, 2026 at 02:16:52PM +0200, Heikki Linnakangas wrote:
>> 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.
There's already this check in RequestNamedLWLockTranche():
if (!process_shmem_requests_in_progress)
elog(FATAL, "cannot request additional LWLocks outside
shmem_request_hook");
shmem_request_hooks are only called early at postmaster startup.
> + SpinLockAcquire(ShmemLock);
> + LocalNumUserDefinedTranches = LWLockTranches->num_user_defined;
> + SpinLockRelease(ShmemLock);
>
> Not critical, but it might be worth making num_user_defined an atomic.
Yeah I considered that. The lock is still needed in
LWLockNewTrancheId(), though, to prevent two concurrent
LWLockNewTrancheId() calls from running concurrently. Using an atomic
would allow the extra optimization of reading the value without
acquiring spinlock, but it seems more clear to have a clear-cut rule
that you must always hold the spinlock whenever accessing the field.
> 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?
Yeah. LWLockAcquire has this:
/*
* We can't wait if we haven't got a PGPROC. This should only occur
* during bootstrap or shared memory initialization. Put an Assert
here
* to catch unsafe coding practices.
*/
Assert(!(proc == NULL && IsUnderPostmaster));
To be honest I didn't realize we tolerate that, calling LWLockAcquire in
postmaster, until I started to work on this. It might be worth having
some extra sanity checks here, to e.g. to throw an error if
LWLockAcquire is called from postmaster after startup. But this isn't new.
> 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?
Makes sense.
Thanks for the review!
- Heikki
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
@ 2026-03-26 16:34 ` Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Sami Imseih @ 2026-03-26 16:34 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
Thanks for the patches!
> I propose the attached refactorings to make this less confusing. See
> commit messages for details.
I only took a look at 0001 so far, and I do agree with this statement
in the commit message:
"The "user defined" term was already used in LWTRANCHE_FIRST_USER_DEFINED,
so let's standardize on that to mean tranches allocated with either
RequestNamedLWLockTranche() or LWLockNewTrancheId()."
I do wonder if 0001 is going far enough though.
Instead of just standardizing that "user defined" could mean tranches allocated
with RequestNamedLWLockTranche() or LWLockNewTrancheId(), how about we also
rename these APIs to reflect that as well? This way we remove all concept of
"named tranche" which is what it sounds like to me you are proposing.
rename RequestNamedLWLockTranche() to RequestUserDefinedLWLockTranche()
and LWLockNewTrancheId() to RegisterUserDefinedLWLockTranche()
RequestNamedLWLockTranche() requests the lwlock at shmem_request time,
which is later registered via LWLockNewTrancheId() when lwlocks are
initialized by the postmaster.
Also, the name LWLockNewTrancheId() is selling what this function does
too short.
It does return a new tranche ID, but it also takes in a user-defined tranche
name and copies ("registers") that name into LWLockTrancheNames.
v19 is already changing the signature of LWLockNewTrancheId(), so maybe
improving the names of these APIs makes sense to do.
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
@ 2026-03-26 16:57 ` Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Heikki Linnakangas @ 2026-03-26 16:57 UTC (permalink / raw)
To: Sami Imseih <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Thanks!
On 26/03/2026 18:34, Sami Imseih wrote:
>> I propose the attached refactorings to make this less confusing. See
>> commit messages for details.
>
> I only took a look at 0001 so far, and I do agree with this statement
> in the commit message:
>
> "The "user defined" term was already used in LWTRANCHE_FIRST_USER_DEFINED,
> so let's standardize on that to mean tranches allocated with either
> RequestNamedLWLockTranche() or LWLockNewTrancheId()."
>
> I do wonder if 0001 is going far enough though.
>
> Instead of just standardizing that "user defined" could mean tranches allocated
> with RequestNamedLWLockTranche() or LWLockNewTrancheId(), how about we also
> rename these APIs to reflect that as well? This way we remove all concept of
> "named tranche" which is what it sounds like to me you are proposing.
>
> rename RequestNamedLWLockTranche() to RequestUserDefinedLWLockTranche()
> and LWLockNewTrancheId() to RegisterUserDefinedLWLockTranche()
I'd rather not change RequestNamedLWLockTranche(), because I think
LWLockNewTrancheId() is better and should be used in new code. I
consider RequestNamedLWLockTranche() to be a legacy function, for
backwards compatibility.
> RequestNamedLWLockTranche() requests the lwlock at shmem_request time,
> which is later registered via LWLockNewTrancheId() when lwlocks are
> initialized by the postmaster.
>
> Also, the name LWLockNewTrancheId() is selling what this function does
> too short.
> It does return a new tranche ID, but it also takes in a user-defined tranche
> name and copies ("registers") that name into LWLockTrancheNames.
>
> v19 is already changing the signature of LWLockNewTrancheId(), so maybe
> improving the names of these APIs makes sense to do.
Oh, I didn't realize we changed the LWLockNewTrancheId() signature!
Yeah, if we're changing it anyway, we might as well rename it. I'm not
sure if I like RegisterUserDefinedLWLockTranche() better, but let's
think it through.
- Heikki
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
@ 2026-03-27 03:57 ` Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Sami Imseih @ 2026-03-27 03:57 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
> > Thanks!
> >
> > On 26/03/2026 18:34, Sami Imseih wrote:
> >>> I propose the attached refactorings to make this less confusing. See
> >>> commit messages for details.
> >>
> >> I only took a look at 0001 so far, and I do agree with this statement
> >> in the commit message:
>
> I committed these now, but I'm all ears if you still have comments on
> the rest of the patches.
Sorry for the delay. I see you committed the rest. The only issue I found
is with d6eba30
+/* backend-local copy of NamedLWLockTranches->num_user_defined */
+static int LocalNumUserDefinedTranches;
The comment here should reference "LWLockTranches->num_user_defined "
instead.
>> rename RequestNamedLWLockTranche() to RequestUserDefinedLWLockTranche()
>> and LWLockNewTrancheId() to RegisterUserDefinedLWLockTranche()
> I'd rather not change RequestNamedLWLockTranche(), because I think
> LWLockNewTrancheId() is better and should be used in new code.
That's fair.
>> v19 is already changing the signature of LWLockNewTrancheId(), so maybe
>> improving the names of these APIs makes sense to do.
> Oh, I didn't realize we changed the LWLockNewTrancheId() signature!
> Yeah, if we're changing it anyway, we might as well rename it. I'm not
> sure if I like RegisterUserDefinedLWLockTranche() better, but let's
> think it through.
Maybe, RegisterNewLWLockTrancheId() could be more meaningful?
Also, there are a few places in lwlock.c where "named tranches" is mentioned.
Maybe we should just say "user-defined tranches" instead?
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
@ 2026-03-27 04:49 ` Sami Imseih <[email protected]>
2026-03-27 14:53 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Sami Imseih @ 2026-03-27 04:49 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
> +/* backend-local copy of NamedLWLockTranches->num_user_defined */
> +static int LocalNumUserDefinedTranches;
> The comment here should reference "LWLockTranches->num_user_defined "
> instead.
> Also, there are a few places in lwlock.c where "named tranches" is mentioned.
> Maybe we should just say "user-defined tranches" instead?
Like the attached.
--
Sami
Attachments:
[application/octet-stream] v1-0001-fix-some-comments-for-lwlock-tranches.patch (2.0K, 2-v1-0001-fix-some-comments-for-lwlock-tranches.patch)
download | inline diff:
From 3f8053adde0da2c99fd1e02f464a7247c73f0887 Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Fri, 27 Mar 2026 04:46:55 +0000
Subject: [PATCH v1 1/1] fix some comments for lwlock tranches
---
src/backend/storage/lmgr/lwlock.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 241f1f08430..2ee385698e5 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -200,7 +200,7 @@ typedef struct LWLockTrancheShmemData
static LWLockTrancheShmemData *LWLockTranches;
-/* backend-local copy of NamedLWLockTranches->num_user_defined */
+/* backend-local copy of LWLockTranches->num_user_defined */
static int LocalNumUserDefinedTranches;
/*
@@ -460,7 +460,7 @@ LWLockShmemInit(void)
}
/*
- * Initialize LWLocks that are fixed and those belonging to named tranches.
+ * Initialize LWLocks that are fixed and those belonging to user-defined tranches.
*/
static void
InitializeLWLocks(int numLocks)
@@ -487,7 +487,7 @@ InitializeLWLocks(int numLocks)
LWLockInitialize(&MainLWLockArray[pos++].lock, LWTRANCHE_PREDICATE_LOCK_MANAGER);
/*
- * Copy the info about any named tranches into shared memory (so that
+ * Copy the info about any user-defined tranches into shared memory (so that
* other processes can see it), and initialize the requested LWLocks.
*/
Assert(pos == NUM_FIXED_LWLOCKS);
@@ -536,8 +536,9 @@ GetNamedLWLockTranche(const char *tranche_name)
/*
* Obtain the position of base address of LWLock belonging to requested
- * tranche_name in MainLWLockArray. LWLocks for named tranches are placed
- * in MainLWLockArray after fixed locks.
+ * tranche_name in MainLWLockArray. LWLocks for user-defined tranches
+ * requested with RequestNamedLWLockTranche() are placed in
+ * MainLWLockArray after fixed locks.
*/
for (int i = 0; i < LocalNumUserDefinedTranches; i++)
{
--
2.47.3
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
@ 2026-03-27 14:53 ` Sami Imseih <[email protected]>
1 sibling, 0 replies; 14+ messages in thread
From: Sami Imseih @ 2026-03-27 14:53 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
> Committed with that little change, thanks!
Thanks!
I think there is one more comment cleanup in lwlock.c
/*
- * This points to the main array of LWLocks in shared memory. Backends inherit
- * the pointer by fork from the postmaster (except in the EXEC_BACKEND case,
- * where we have special measures to pass it down).
+ * This points to the main array of LWLocks in shared memory.
*/
we no longer need to take special measures to pass down MainLWLockArray
through the BackendParameters.
--
Sami
Attachments:
[application/octet-stream] v1-0001-Remove-another-outdated-comment-regading-MainLWLo.patch (990B, 2-v1-0001-Remove-another-outdated-comment-regading-MainLWLo.patch)
download | inline diff:
From e13f8683b3fc4ef08ad7892e8f6b55c74ab52fc5 Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Fri, 27 Mar 2026 14:49:02 +0000
Subject: [PATCH v1 1/1] Remove another outdated comment regading
MainLWLockArray
---
src/backend/storage/lmgr/lwlock.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 7a68071302a..ade5c652e37 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -146,9 +146,7 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) ==
"missing entries in BuiltinTrancheNames[]");
/*
- * This points to the main array of LWLocks in shared memory. Backends inherit
- * the pointer by fork from the postmaster (except in the EXEC_BACKEND case,
- * where we have special measures to pass it down).
+ * This points to the main array of LWLocks in shared memory.
*/
LWLockPadded *MainLWLockArray = NULL;
--
2.47.3
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
@ 2026-03-27 21:22 ` Andres Freund <[email protected]>
2026-03-27 21:50 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Andres Freund @ 2026-03-27 21:22 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Sami Imseih <[email protected]>; Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
On 2026-03-27 11:45:56 +0200, Heikki Linnakangas wrote:
> Committed with that little change, thanks!
This seems to have broken buildfarm animal batta:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=batta&dt=2026-03-27%2002%3A05%3A01
# Running: pg_rewind --debug --source-pgdata /home/admin/batta/buildroot/HEAD/pgsql.build/src/bin/pg_rewind/tmp_check/t_001_basic_standby_local_data/pgdata --target-pgdata /home/admin/batta/buildroot/HEAD/pgsql.build/src/bin/pg_rewind/tmp_check/t_001_basic_primary_local_data/pgdata --no-sync --config-file /home/admin/batta/buildroot/HEAD/pgsql.build/src/bin/pg_rewind/tmp_check/tmp_test_QbsG/primary-postgresql.conf.tmp
pg_rewind: executing "/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres" for target server to complete crash recovery
TRAP: failed Assert("MemoryContextIsValid(context)"), File: "mcxt.c", Line: 1270, PID: 230491
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(ExceptionalCondition+0x54)[0xaaaae186c204]
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(MemoryContextAllocExtended+0x0)[0xaaaae18a2a24]
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(RequestNamedLWLockTranche+0x6c)[0xaaaae16e7310]
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(process_shmem_requests+0x28)[0xaaaae1881628]
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(PostgresSingleUserMain+0xc4)[0xaaaae1701a34]
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(main+0x6ac)[0xaaaae12a2adc]
/lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe8)[0xffff99713dd8]
/home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres(+0xf2b98)[0xaaaae12a2b98]
Aborted
pg_rewind: error: postgres single-user mode in target cluster failed
pg_rewind: detail: Command was: /home/admin/batta/buildroot/HEAD/pgsql.build/tmp_install/home/admin/batta/buildroot/HEAD/inst/bin/postgres --single -F -D /home/admin/batta/buildroot/HEAD/pgsql.build/src/bin/pg_rewind/tmp_check/t_001_basic_primary_local_data/pgdata -c config_file=/home/admin/batta/buildroot/HEAD/pgsql.build/src/bin/pg_rewind/tmp_check/tmp_test_QbsG/primary-postgresql.conf.tmp template1 < /dev/null
Presumably the reason that batta failed is its special configuration:
shared_preload_libraries = 'pg_stat_statements'; regress_dump_restore; wal_consistency_checking; compute_query_id = regress; --enable-injection-points
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
@ 2026-03-27 21:50 ` Nathan Bossart <[email protected]>
2026-03-27 22:05 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Nathan Bossart @ 2026-03-27 21:50 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; Sami Imseih <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Fri, Mar 27, 2026 at 05:22:33PM -0400, Andres Freund wrote:
> TRAP: failed Assert("MemoryContextIsValid(context)"), File: "mcxt.c", Line: 1270, PID: 230491
> [...](ExceptionalCondition+0x54)[0xaaaae186c204]
> [...](MemoryContextAllocExtended+0x0)[0xaaaae18a2a24]
> [...](RequestNamedLWLockTranche+0x6c)[0xaaaae16e7310]
> [...](process_shmem_requests+0x28)[0xaaaae1881628]
> [...](PostgresSingleUserMain+0xc4)[0xaaaae1701a34]
> [...](main+0x6ac)[0xaaaae12a2adc]
> /lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe8)[0xffff99713dd8]
> [...](+0xf2b98)[0xaaaae12a2b98]
> Aborted
> pg_rewind: error: postgres single-user mode in target cluster failed
Hm. AFAICT PostmasterContext isn't created in single-user mode, and the
commit in question has RequestNamedLWLockTranche() allocate requests there.
I guess the idea is to allow backends to free that memory after forking
from postmaster, but we don't do that for the NamedLWLockTrancheRequests
list. Maybe we should surround the last part of that function with
MemoryContextSwitchTo(...) to either TopMemoryContext or PostmasterContext
depending on whether we're in single-user mode.
--
nathan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
2026-03-27 21:50 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
@ 2026-03-27 22:05 ` Nathan Bossart <[email protected]>
2026-03-27 22:10 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Nathan Bossart @ 2026-03-27 22:05 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; Sami Imseih <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Fri, Mar 27, 2026 at 04:50:12PM -0500, Nathan Bossart wrote:
> On Fri, Mar 27, 2026 at 05:22:33PM -0400, Andres Freund wrote:
>> TRAP: failed Assert("MemoryContextIsValid(context)"), File: "mcxt.c", Line: 1270, PID: 230491
>> [...](ExceptionalCondition+0x54)[0xaaaae186c204]
>> [...](MemoryContextAllocExtended+0x0)[0xaaaae18a2a24]
>> [...](RequestNamedLWLockTranche+0x6c)[0xaaaae16e7310]
>> [...](process_shmem_requests+0x28)[0xaaaae1881628]
>> [...](PostgresSingleUserMain+0xc4)[0xaaaae1701a34]
>> [...](main+0x6ac)[0xaaaae12a2adc]
>> /lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe8)[0xffff99713dd8]
>> [...](+0xf2b98)[0xaaaae12a2b98]
>> Aborted
>> pg_rewind: error: postgres single-user mode in target cluster failed
>
> Hm. AFAICT PostmasterContext isn't created in single-user mode, and the
> commit in question has RequestNamedLWLockTranche() allocate requests there.
> I guess the idea is to allow backends to free that memory after forking
> from postmaster, but we don't do that for the NamedLWLockTrancheRequests
> list. Maybe we should surround the last part of that function with
> MemoryContextSwitchTo(...) to either TopMemoryContext or PostmasterContext
> depending on whether we're in single-user mode.
Concretely, like the attached.
--
nathan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
2026-03-27 21:50 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:05 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
@ 2026-03-27 22:10 ` Nathan Bossart <[email protected]>
2026-03-27 23:02 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Nathan Bossart @ 2026-03-27 22:10 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Andres Freund <[email protected]>; Sami Imseih <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Sat, Mar 28, 2026 at 12:07:26AM +0200, Heikki Linnakangas wrote:
> LGTM, thanks! Will you commit or want me to pick it up?
I'm not able to commit it right this second, so feel free to take it. Else
it'll probably be a day or two before I can get to it.
--
nathan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
2026-03-27 21:50 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:05 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:10 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
@ 2026-03-27 23:02 ` Heikki Linnakangas <[email protected]>
2026-03-28 17:20 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Heikki Linnakangas @ 2026-03-27 23:02 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; Sami Imseih <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On 28/03/2026 00:10, Nathan Bossart wrote:
> On Sat, Mar 28, 2026 at 12:07:26AM +0200, Heikki Linnakangas wrote:
>> LGTM, thanks! Will you commit or want me to pick it up?
>
> I'm not able to commit it right this second, so feel free to take it. Else
> it'll probably be a day or two before I can get to it.
Ok, committed, thanks!
- Heikki
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
2026-03-27 21:50 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:05 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:10 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 23:02 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
@ 2026-03-28 17:20 ` Sami Imseih <[email protected]>
2026-03-30 14:18 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Sami Imseih @ 2026-03-28 17:20 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi Heikki,
Just raising this again to make sure it doesn’t get overlooked [1].
Thanks!
[1] [https://www.postgresql.org/message-id/CAA5RZ0vPWNMvTBqyH7nqDRrHd6Y4Et5iNqXFuwpbsPOk3cL4rQ%40mail.gma...]
--
Sami
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Clean up NamedLWLockTranche stuff
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-26 16:57 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 04:49 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
2026-03-27 21:22 ` Re: Clean up NamedLWLockTranche stuff Andres Freund <[email protected]>
2026-03-27 21:50 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:05 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 22:10 ` Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-27 23:02 ` Re: Clean up NamedLWLockTranche stuff Heikki Linnakangas <[email protected]>
2026-03-28 17:20 ` Re: Clean up NamedLWLockTranche stuff Sami Imseih <[email protected]>
@ 2026-03-30 14:18 ` Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Heikki Linnakangas @ 2026-03-30 14:18 UTC (permalink / raw)
To: Sami Imseih <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On 28/03/2026 19:20, Sami Imseih wrote:
> Hi Heikki,
>
> Just raising this again to make sure it doesn’t get overlooked [1].
Fixed, thanks!
- Heikki
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2026-03-30 14:18 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-26 14:37 Re: Clean up NamedLWLockTranche stuff Nathan Bossart <[email protected]>
2026-03-26 15:21 ` Heikki Linnakangas <[email protected]>
2026-03-26 16:34 ` Sami Imseih <[email protected]>
2026-03-26 16:57 ` Heikki Linnakangas <[email protected]>
2026-03-27 03:57 ` Sami Imseih <[email protected]>
2026-03-27 04:49 ` Sami Imseih <[email protected]>
2026-03-27 14:53 ` Sami Imseih <[email protected]>
2026-03-27 21:22 ` Andres Freund <[email protected]>
2026-03-27 21:50 ` Nathan Bossart <[email protected]>
2026-03-27 22:05 ` Nathan Bossart <[email protected]>
2026-03-27 22:10 ` Nathan Bossart <[email protected]>
2026-03-27 23:02 ` Heikki Linnakangas <[email protected]>
2026-03-28 17:20 ` Sami Imseih <[email protected]>
2026-03-30 14:18 ` Heikki Linnakangas <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox