On Thu, Nov 18, 2021 at 5:01 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
> The following is what I made up in my mind after looking at other
> existing messages, like [1] and the review comments:
> errmsg("cannot send signal to postmaster %d", pid, --> the process
> is postmaster but the caller isn't allowed to signal.
> errmsg("cannot send signal to PostgreSQL server process %d", pid,
> --> the process is a postgresql process but the caller isn't allowed
> to signal.
> errmsg("PID %d is not a PostgreSQL backend process", pid, ---> it may
> be another postgres processes like syslogger or stats collector or
> non-postgres process but not a backend process.
>
> Thoughts?
>
> [1]
> (errmsg("could not send signal to process %d: %m", pid)));
> (errmsg("failed to send signal to postmaster: %m")));
Here's the v4 patch with the above changes, the output looks like [1].
Please review it further.
[1]
postgres=# select pg_terminate_backend(2407245);
WARNING: cannot send signal to postmaster 2407245
pg_terminate_backend
----------------------
f
(1 row)
postgres=# select pg_terminate_backend(2407246);
WARNING: cannot send signal to PostgreSQL server process 2407246
pg_terminate_backend
----------------------
f
(1 row)
postgres=# select pg_terminate_backend(2407286);
WARNING: PID 2407286 is not a PostgreSQL backend process
pg_terminate_backend
----------------------
f
(1 row)
Regards,
Bharath Rupireddy.
From 24800ffa999f527ac9c526ef11e6679d18269d39 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Fri, 19 Nov 2021 02:09:49 +0000
Subject: [PATCH v4] Improve PID XXXX is not a PostgreSQL server process
message
Currently, pg_signal_backend emits generic WARNING "PID XXXX is
not a PostgreSQL server process" for postmaster and auxiliary
processes which doesn't sound sensible. This patch tries to
improve this message.
---
src/backend/storage/ipc/signalfuncs.c | 38 +++++++++++++++++++++++----
1 file changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c
index de69d60e79..8d0f0fea65 100644
--- a/src/backend/storage/ipc/signalfuncs.c
+++ b/src/backend/storage/ipc/signalfuncs.c
@@ -48,7 +48,17 @@
static int
pg_signal_backend(int pid, int sig)
{
- PGPROC *proc = BackendPidGetProc(pid);
+ PGPROC *proc;
+
+ if (PostmasterPid == pid)
+ {
+ ereport(WARNING,
+ (errmsg("cannot send signal to postmaster %d", pid)));
+
+ return SIGNAL_BACKEND_ERROR;
+ }
+
+ proc = BackendPidGetProc(pid);
/*
* BackendPidGetProc returns NULL if the pid isn't valid; but by the time
@@ -61,11 +71,29 @@ pg_signal_backend(int pid, int sig)
if (proc == NULL)
{
/*
- * This is just a warning so a loop-through-resultset will not abort
- * if one backend terminated on its own during the run.
+ * AuxiliaryProcs are still PostgreSQL server processes, do a little
+ * more work and report a proper warning that says we cannot signal
+ * them.
+ *
+ * For an auxiliary process, retrieve process info from AuxiliaryProcs
+ * stored in shared-memory.
*/
- ereport(WARNING,
- (errmsg("PID %d is not a PostgreSQL server process", pid)));
+ proc = AuxiliaryPidGetProc(pid);
+
+ if (proc)
+ ereport(WARNING,
+ (errmsg("cannot send signal to PostgreSQL server process %d",
+ pid)));
+ else
+ {
+ /*
+ * This is just a warning so a loop-through-resultset will not
+ * abort if one backend terminated on its own during the run.
+ */
+ ereport(WARNING,
+ (errmsg("PID %d is not a PostgreSQL backend process", pid)));
+ }
+
return SIGNAL_BACKEND_ERROR;
}
--
2.25.1
