public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bharath Rupireddy <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Avoid multiple SetLatch() calls in procsignal_sigusr1_handler()
Date: Tue, 28 Feb 2023 21:00:00 +0530
Message-ID: <CALj2ACWd5apddj6Cd885WwJ6LquYu_G81C4GoR4xSoDV1x-FEA@mail.gmail.com> (raw)
Hi,
Most of the multiplexed SIGUSR1 handlers are setting latch explicitly
when the procsignal_sigusr1_handler() can do that for them at the end.
These multiplexed handlers are currently being used as SIGUSR1
handlers, not as independent handlers, so no problem if SetLatch() is
removed from them. A few others do it right by saying /* latch will be
set by procsignal_sigusr1_handler */. Although, calling SetLatch() in
quick succession does no harm (it just returns if the latch was
previously set), it seems unnecessary.
I'm attaching a patch that avoids multiple SetLatch() calls.
Thoughts?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v1-0001-Avoid-multiple-SetLatch-calls-in-procsignal_sigus.patch (3.7K, 2-v1-0001-Avoid-multiple-SetLatch-calls-in-procsignal_sigus.patch)
download | inline diff:
From 128d45f53606aaeac17cdfd43df801ec6d7a3e50 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Tue, 28 Feb 2023 12:11:39 +0000
Subject: [PATCH v1] Avoid multiple SetLatch() calls in
procsignal_sigusr1_handler()
---
src/backend/access/transam/parallel.c | 2 +-
src/backend/commands/async.c | 4 +---
.../replication/logical/applyparallelworker.c | 2 +-
src/backend/replication/walsender.c | 2 ++
src/backend/storage/ipc/sinval.c | 4 +---
src/backend/tcop/postgres.c | 12 ++++--------
6 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index b26f2a64fb..e1b48295ed 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1015,7 +1015,7 @@ HandleParallelMessageInterrupt(void)
{
InterruptPending = true;
ParallelMessagePending = true;
- SetLatch(MyLatch);
+ /* latch will be set by procsignal_sigusr1_handler */
}
/*
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index ef909cf4e0..ad533db655 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -1859,9 +1859,7 @@ HandleNotifyInterrupt(void)
/* signal that work needs to be done */
notifyInterruptPending = true;
-
- /* make sure the event is processed in due course */
- SetLatch(MyLatch);
+ /* latch will be set by procsignal_sigusr1_handler */
}
/*
diff --git a/src/backend/replication/logical/applyparallelworker.c b/src/backend/replication/logical/applyparallelworker.c
index 4518683779..6db40cc121 100644
--- a/src/backend/replication/logical/applyparallelworker.c
+++ b/src/backend/replication/logical/applyparallelworker.c
@@ -987,7 +987,7 @@ HandleParallelApplyMessageInterrupt(void)
{
InterruptPending = true;
ParallelApplyMessagePending = true;
- SetLatch(MyLatch);
+ /* latch will be set by procsignal_sigusr1_handler */
}
/*
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75e8363e24..f063a712d4 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3210,6 +3210,8 @@ HandleWalSndInitStopping(void)
kill(MyProcPid, SIGTERM);
else
got_STOPPING = true;
+
+ /* latch will be set by procsignal_sigusr1_handler */
}
/*
diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c
index b405f08313..bd6adcef53 100644
--- a/src/backend/storage/ipc/sinval.c
+++ b/src/backend/storage/ipc/sinval.c
@@ -161,9 +161,7 @@ HandleCatchupInterrupt(void)
*/
catchupInterruptPending = true;
-
- /* make sure the event is processed in due course */
- SetLatch(MyLatch);
+ /* latch will be set by procsignal_sigusr1_handler */
}
/*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index cab709b07b..2a3477dc80 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3007,8 +3007,6 @@ FloatExceptionHandler(SIGNAL_ARGS)
void
RecoveryConflictInterrupt(ProcSignalReason reason)
{
- int save_errno = errno;
-
/*
* Don't joggle the elbow of proc_exit
*/
@@ -3123,13 +3121,11 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
}
/*
- * Set the process latch. This function essentially emulates signal
- * handlers like die() and StatementCancelHandler() and it seems prudent
- * to behave similarly as they do.
+ * Latch will be set by procsignal_sigusr1_handler. This function
+ * essentially emulates signal handlers like die() and
+ * StatementCancelHandler() and it seems prudent to behave similarly as
+ * they do.
*/
- SetLatch(MyLatch);
-
- errno = save_errno;
}
/*
--
2.34.1
view thread (9+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Avoid multiple SetLatch() calls in procsignal_sigusr1_handler()
In-Reply-To: <CALj2ACWd5apddj6Cd885WwJ6LquYu_G81C4GoR4xSoDV1x-FEA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox