Use LW_SHARED in WakeupWalSummarizer() for WALSummarizerLock lock
Hi all,
While reading walsummarizer.c code, I noticed that in
WakeupWalSummarizer() we acquire the WALSummarizerLock lock in
LW_EXCLUSIVE mode despite only reading
WalSummarizerCtl->summarizer_pgprocno. The attached patch uses
LW_SHARED mode instead. Feedback is very welcome.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Use-LW_SHARED-in-WakeupWalSummarizer-for-WALSumma.patchapplication/octet-stream; name=v1-0001-Use-LW_SHARED-in-WakeupWalSummarizer-for-WALSumma.patchDownload
From c75a8227bb1c11f1f6d71de6742ee950e13113d5 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 25 Aug 2025 15:36:20 -0700
Subject: [PATCH v1] Use LW_SHARED in WakeupWalSummarizer() for
WALSummarizerLock lock.
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch-through:
---
src/backend/postmaster/walsummarizer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index 777c9a8d555..fa930c4f4d6 100644
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -644,7 +644,7 @@ WakeupWalSummarizer(void)
if (WalSummarizerCtl == NULL)
return;
- LWLockAcquire(WALSummarizerLock, LW_EXCLUSIVE);
+ LWLockAcquire(WALSummarizerLock, LW_SHARED);
pgprocno = WalSummarizerCtl->summarizer_pgprocno;
LWLockRelease(WALSummarizerLock);
--
2.47.3
On Mon, Aug 25, 2025 at 03:38:13PM -0700, Masahiko Sawada wrote:
While reading walsummarizer.c code, I noticed that in
WakeupWalSummarizer() we acquire the WALSummarizerLock lock in
LW_EXCLUSIVE mode despite only reading
WalSummarizerCtl->summarizer_pgprocno. The attached patch uses
LW_SHARED mode instead. Feedback is very welcome.
You could probably do something similar for WaitForWalSummarization().
--
nathan
On Mon, Aug 25, 2025 at 5:08 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Aug 25, 2025 at 03:38:13PM -0700, Masahiko Sawada wrote:
While reading walsummarizer.c code, I noticed that in
WakeupWalSummarizer() we acquire the WALSummarizerLock lock in
LW_EXCLUSIVE mode despite only reading
WalSummarizerCtl->summarizer_pgprocno. The attached patch uses
LW_SHARED mode instead. Feedback is very welcome.You could probably do something similar for WaitForWalSummarization().
Good point. I've attached the updated patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Use-LW_SHARED-in-walsummarizer.c-for-WALSummarize.patchapplication/octet-stream; name=v2-0001-Use-LW_SHARED-in-walsummarizer.c-for-WALSummarize.patchDownload
From 13a736ad884ce7713d0ffe677a8f9a0d0ef01575 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 25 Aug 2025 15:36:20 -0700
Subject: [PATCH v2] Use LW_SHARED in walsummarizer.c for WALSummarizerLock
lock where possible.
Previously, we used LW_EXCLUSIVE in several places despite only reading
WalSummarizerCtl fields. This patch reduces the lock level to LW_SHARED
where we are only reading the shared fields.
Backpatch to 17, where wal summarization was introduced.
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDdKhf_9oriEYxY-JCdF+Oe_muhca3pcdkMEdBMzyHyKw@mail.gmail.com
Backpatch-through: 17
---
src/backend/postmaster/walsummarizer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index 777c9a8d555..e1f142f20c7 100644
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -644,7 +644,7 @@ WakeupWalSummarizer(void)
if (WalSummarizerCtl == NULL)
return;
- LWLockAcquire(WALSummarizerLock, LW_EXCLUSIVE);
+ LWLockAcquire(WALSummarizerLock, LW_SHARED);
pgprocno = WalSummarizerCtl->summarizer_pgprocno;
LWLockRelease(WALSummarizerLock);
@@ -685,7 +685,7 @@ WaitForWalSummarization(XLogRecPtr lsn)
/*
* If the LSN summarized on disk has reached the target value, stop.
*/
- LWLockAcquire(WALSummarizerLock, LW_EXCLUSIVE);
+ LWLockAcquire(WALSummarizerLock, LW_SHARED);
summarized_lsn = WalSummarizerCtl->summarized_lsn;
pending_lsn = WalSummarizerCtl->pending_lsn;
LWLockRelease(WALSummarizerLock);
--
2.47.3
On Mon, Aug 25, 2025 at 5:48 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Mon, Aug 25, 2025 at 5:08 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Aug 25, 2025 at 03:38:13PM -0700, Masahiko Sawada wrote:
While reading walsummarizer.c code, I noticed that in
WakeupWalSummarizer() we acquire the WALSummarizerLock lock in
LW_EXCLUSIVE mode despite only reading
WalSummarizerCtl->summarizer_pgprocno. The attached patch uses
LW_SHARED mode instead. Feedback is very welcome.You could probably do something similar for WaitForWalSummarization().
Good point. I've attached the updated patch.
Pushed.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com