From 67790ed15f6afeea8890f17407af87dd3d5347ef Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 13 Jun 2018 10:23:07 +0200 Subject: [PATCH 2/2] Support optional message in backend cancel/terminate This adds the ability for the caller of pg_terminate_backend() or pg_cancel_backend() to include an optional message to the process which is being signalled. The message will be appended to the error message returned to the killed or cancelled process. The new syntax overloaded the existing as: SELECT pg_terminate_backend( [, msg]); SELECT pg_cancel_backend( [, msg]); The backend API also expose functionality for altering the errcode used when terminating or canceling the backend. --- doc/src/sgml/func.sgml | 12 +- src/backend/storage/ipc/backend_signal.c | 320 ++++++++++++++++++++++++++++-- src/backend/storage/ipc/ipci.c | 3 + src/backend/tcop/postgres.c | 55 ++++- src/backend/utils/init/postinit.c | 2 + src/include/catalog/pg_proc.dat | 6 + src/include/storage/backend_signal.h | 27 +++ src/test/regress/expected/admin_funcs.out | 29 +++ src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/admin_funcs.sql | 7 + 10 files changed, 442 insertions(+), 21 deletions(-) create mode 100644 src/include/storage/backend_signal.h create mode 100644 src/test/regress/expected/admin_funcs.out create mode 100644 src/test/regress/sql/admin_funcs.sql diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index b851fe023a..ad9763698f 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -18540,7 +18540,7 @@ SELECT set_config('log_statement_stats', 'off', false); - pg_cancel_backend(pid int) + pg_cancel_backend(pid int [, message text]) boolean Cancel a backend's current query. This is also allowed if the @@ -18565,7 +18565,7 @@ SELECT set_config('log_statement_stats', 'off', false); - pg_terminate_backend(pid int) + pg_terminate_backend(pid int [, message text]) boolean Terminate a backend. This is also allowed if the calling role @@ -18596,6 +18596,14 @@ SELECT set_config('log_statement_stats', 'off', false); The role of an active backend can be found from the usename column of the pg_stat_activity view. + If the optional message parameter is set, the text + will be appended to the error message returned to the signalled backend. + message is limited to 128 bytes, any longer text + will be truncated. An example where we cancel our own backend: + +postgres=# SELECT pg_cancel_backend(pg_backend_pid(), 'Cancellation message text'); +ERROR: canceling statement due to user request: Cancellation message text + diff --git a/src/backend/storage/ipc/backend_signal.c b/src/backend/storage/ipc/backend_signal.c index 2b81c161d9..69a7f106d4 100644 --- a/src/backend/storage/ipc/backend_signal.c +++ b/src/backend/storage/ipc/backend_signal.c @@ -18,20 +18,45 @@ #include "catalog/pg_authid.h" #include "funcapi.h" +#include "mb/pg_wchar.h" #include "miscadmin.h" #include "postmaster/syslogger.h" +#include "storage/backend_signal.h" +#include "storage/ipc.h" #include "storage/pmsignal.h" #include "storage/proc.h" #include "storage/procarray.h" #include "utils/acl.h" #include "utils/builtins.h" +/* + * Structure for registering a feedback payload to be sent to a cancelled, or + * terminated backend. Each backend is registered per pid in the array which is + * indexed by Backend ID. Reading and writing the message is protected by a + * per-slot spinlock. + */ +typedef struct +{ + pid_t pid; + slock_t mutex; + char message[MAX_CANCEL_MSG]; + int len; + int sqlerrcode; +} BackendSignalFeedbackShmemStruct; + +static BackendSignalFeedbackShmemStruct *BackendSignalFeedbackSlots = NULL; +static volatile BackendSignalFeedbackShmemStruct *MyCancelSlot = NULL; +static void CleanupBackendSignalFeedback(int status, Datum argument); +static int backend_feedback(pid_t backend_pid, char *message, int sqlerrcode); + /* * Send a signal to another backend. * - * The signal is delivered if the user is either a superuser or the same - * role as the backend being signaled. For "dangerous" signals, an explicit - * check for superuser needs to be done prior to calling this function. + * The signal is delivered if the user is either a superuser or the same role + * as the backend being signaled. For "dangerous" signals, an explicit check + * for superuser needs to be done prior to calling this function. If msg is + * set, the contents will be passed as a message to the backend in the error + * message. * * Returns 0 on success, 1 on general failure, 2 on normal permission error * and 3 if the caller needs to be a superuser. @@ -45,7 +70,7 @@ #define SIGNAL_BACKEND_NOPERMISSION 2 #define SIGNAL_BACKEND_NOSUPERUSER 3 static int -pg_signal_backend(int pid, int sig) +pg_signal_backend(int pid, int sig, char *msg) { PGPROC *proc = BackendPidGetProc(pid); @@ -77,6 +102,15 @@ pg_signal_backend(int pid, int sig) !has_privs_of_role(GetUserId(), DEFAULT_ROLE_SIGNAL_BACKENDID)) return SIGNAL_BACKEND_NOPERMISSION; + /* If the user supplied a message to the signalled backend */ + if (msg != NULL) + { + if (sig == SIGINT) + SetBackendCancelMessage(pid, msg); + else + SetBackendTerminationMessage(pid, msg); + } + /* * Can the process we just validated above end, followed by the pid being * recycled for a new process, before reaching here? Then we'd be trying @@ -107,10 +141,10 @@ pg_signal_backend(int pid, int sig) * * Note that only superusers can signal superuser-owned processes. */ -Datum -pg_cancel_backend(PG_FUNCTION_ARGS) +static bool +pg_cancel_backend_internal(pid_t pid, char *msg) { - int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT); + int r = pg_signal_backend(pid, SIGINT, msg); if (r == SIGNAL_BACKEND_NOSUPERUSER) ereport(ERROR, @@ -125,16 +159,39 @@ pg_cancel_backend(PG_FUNCTION_ARGS) PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS); } +Datum +pg_cancel_backend(PG_FUNCTION_ARGS) +{ + PG_RETURN_BOOL(pg_cancel_backend_internal(PG_GETARG_INT32(0), NULL)); +} + +Datum +pg_cancel_backend_msg(PG_FUNCTION_ARGS) +{ + pid_t pid; + char *msg = NULL; + + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + pid = PG_GETARG_INT32(0); + + if (PG_NARGS() == 2 && !PG_ARGISNULL(1)) + msg = text_to_cstring(PG_GETARG_TEXT_PP(1)); + + PG_RETURN_BOOL(pg_cancel_backend_internal(pid, msg)); +} + /* * Signal to terminate a backend process. This is allowed if you are a member * of the role whose process is being terminated. * * Note that only superusers can signal superuser-owned processes. */ -Datum -pg_terminate_backend(PG_FUNCTION_ARGS) +static bool +pg_terminate_backend_internal(pid_t pid, char *msg) { - int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM); + int r = pg_signal_backend(pid, SIGTERM, msg); if (r == SIGNAL_BACKEND_NOSUPERUSER) ereport(ERROR, @@ -146,7 +203,30 @@ pg_terminate_backend(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend")))); - PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS); + return (r == SIGNAL_BACKEND_SUCCESS); +} + +Datum +pg_terminate_backend(PG_FUNCTION_ARGS) +{ + PG_RETURN_BOOL(pg_terminate_backend_internal(PG_GETARG_INT32(0), NULL)); +} + +Datum +pg_terminate_backend_msg(PG_FUNCTION_ARGS) +{ + pid_t pid; + char *msg = NULL; + + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + pid = PG_GETARG_INT32(0); + + if (PG_NARGS() == 2 && !PG_ARGISNULL(1)) + msg = text_to_cstring(PG_GETARG_TEXT_PP(1)); + + PG_RETURN_BOOL(pg_terminate_backend_internal(pid, msg)); } /* @@ -168,7 +248,6 @@ pg_reload_conf(PG_FUNCTION_ARGS) PG_RETURN_BOOL(true); } - /* * Rotate log file * @@ -214,3 +293,220 @@ pg_rotate_logfile_v2(PG_FUNCTION_ARGS) PG_RETURN_BOOL(true); } +/* + * The following routines handle registering an optional message when + * cancelling, or terminating, a backend as well changing the sqlerrcode used. + * The combined payload of message/errcode is referred to as feedback. The + * message will be stored in shared memory and is limited to MAX_CANCEL_MSG + * characters including the NULL terminator. + * + * Access to the feedback slots is protected by spinlocks. + */ + +/* + * Return the required size for the cancelation feedback Shmem area. + */ +Size +BackendSignalFeedbackShmemSize(void) +{ + return MaxBackends * sizeof(BackendSignalFeedbackShmemStruct); +} + +/* + * Create and initialize the Shmem structure for holding the feedback, the + * bookkeeping for them and the spinlocks associated. + */ +void +BackendSignalFeedbackShmemInit(void) +{ + Size size = BackendSignalFeedbackShmemSize(); + bool found; + int i; + + BackendSignalFeedbackSlots = (BackendSignalFeedbackShmemStruct *) + ShmemInitStruct("BackendSignalFeedbackSlots", size, &found); + + if (!found) + { + MemSet(BackendSignalFeedbackSlots, 0, size); + + for (i = 0; i < MaxBackends; i++) + SpinLockInit(&(BackendSignalFeedbackSlots[i].mutex)); + } +} + +/* + * Set up the slot for the current backend_id + */ +void +BackendSignalFeedbackInit(int backend_id) +{ + volatile BackendSignalFeedbackShmemStruct *slot; + + slot = &BackendSignalFeedbackSlots[backend_id - 1]; + + slot->message[0] = '\0'; + slot->len = 0; + slot->sqlerrcode = 0; + slot->pid = MyProcPid; + + MyCancelSlot = slot; + + on_shmem_exit(CleanupBackendSignalFeedback, Int32GetDatum(backend_id)); +} + +/* + * Ensure that the slot is purged and emptied at exit. Any message gets + * overwritten with null chars to avoid risking exposing a message intended for + * another backend to a new backend. + */ +static void +CleanupBackendSignalFeedback(int status, Datum argument) +{ + int backend_id = DatumGetInt32(argument); + volatile BackendSignalFeedbackShmemStruct *slot; + + slot = &BackendSignalFeedbackSlots[backend_id - 1]; + + Assert(slot == MyCancelSlot); + + MyCancelSlot = NULL; + + if (slot->len > 0) + MemSet(slot->message, '\0', sizeof(slot->message)); + + slot->len = 0; + slot->sqlerrcode = 0; + slot->pid = 0; +} + +/* + * Set a message for the cancellation of the backend with the specified pid, + * using the default sqlerrcode. + */ +int +SetBackendCancelMessage(pid_t backend_pid, char *message) +{ + return backend_feedback(backend_pid, message, ERRCODE_QUERY_CANCELED); +} + +/* + * Set a message for the termination of the backend with the specified pid, + * using the default sqlerrcode. + */ +int +SetBackendTerminationMessage(pid_t backend_pid, char *message) +{ + return backend_feedback(backend_pid, message, ERRCODE_ADMIN_SHUTDOWN); +} + +/* + * Set both a message and a sqlerrcode for use when signalling the backend + * with the specified pid. + */ +int +SetBackendSignalFeedback(pid_t backend_pid, char *message, int sqlerrcode) +{ + return backend_feedback(backend_pid, message, sqlerrcode); +} + +/* + * Sets a cancellation message for the backend with the specified pid, and + * returns the length of the message actually created. If the returned length + * is less than the length of the message parameter, truncation has occurred. + * If the backend isn't found, -1 is returned. If no message is passed, zero is + * returned. If two backends collide in setting a message, the existing message + * will be overwritten by the last one in. + */ +static int +backend_feedback(pid_t backend_pid, char *message, int sqlerrcode) +{ + int i; + int len; + + if (!message) + return 1; + + len = pg_mbcliplen(message, strlen(message), MAX_CANCEL_MSG - 1); + + for (i = 0; i < MaxBackends; i++) + { + BackendSignalFeedbackShmemStruct *slot = &BackendSignalFeedbackSlots[i]; + + if (slot->pid != 0 && slot->pid == backend_pid) + { + SpinLockAcquire(&slot->mutex); + if (slot->pid != backend_pid) + { + SpinLockRelease(&slot->mutex); + goto error; + } + + MemSet(slot->message, '\0', sizeof(slot->message)); + memcpy(slot->message, message, len); + slot->len = len; + slot->sqlerrcode = sqlerrcode; + SpinLockRelease(&slot->mutex); + + if (len != strlen(message)) + ereport(NOTICE, + (errmsg("message is too long and has been truncated"))); + return 0; + } + } + +error: + + elog(LOG, "Cancellation message requested for missing backend %d by %d", + (int) backend_pid, MyProcPid); + + return 1; +} + +/* + * Test whether there is feedback registered for the current backend that can + * be consumed and presented to the user. + */ +bool +HasBackendSignalFeedback(void) +{ + volatile BackendSignalFeedbackShmemStruct *slot = MyCancelSlot; + bool has_message = false; + + if (slot != NULL) + { + SpinLockAcquire(&slot->mutex); + has_message = ((slot->len > 0) && (slot->sqlerrcode != 0)); + SpinLockRelease(&slot->mutex); + } + + return has_message; +} + +/* + * Return the configured signal feedback. The length of the message is returned + * in buf_len. The original length of the message is returned, or zero in case + * no message was found. If the returned length is greater than the size of the passed + * buffer, truncation has been performed. The feedback (message and errcode) is + * cleared on reading. + */ +int +ConsumeBackendSignalFeedback(char *buffer, size_t buf_len, int *sqlerrcode) +{ + volatile BackendSignalFeedbackShmemStruct *slot = MyCancelSlot; + int msg_length = 0; + + if (slot != NULL && slot->len > 0) + { + SpinLockAcquire(&slot->mutex); + strlcpy(buffer, (const char *) slot->message, buf_len); + msg_length = slot->len; + *sqlerrcode = slot->sqlerrcode; + slot->len = 0; + slot->message[0] = '\0'; + slot->sqlerrcode = 0; + SpinLockRelease(&slot->mutex); + } + + return msg_length; +} diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c index 0c86a581c0..5d91450887 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -33,6 +33,7 @@ #include "replication/walreceiver.h" #include "replication/walsender.h" #include "replication/origin.h" +#include "storage/backend_signal.h" #include "storage/bufmgr.h" #include "storage/dsm.h" #include "storage/ipc.h" @@ -150,6 +151,7 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port) size = add_size(size, SyncScanShmemSize()); size = add_size(size, AsyncShmemSize()); size = add_size(size, BackendRandomShmemSize()); + size = add_size(size, BackendSignalFeedbackShmemSize()); #ifdef EXEC_BACKEND size = add_size(size, ShmemBackendArraySize()); #endif @@ -270,6 +272,7 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port) SyncScanShmemInit(); AsyncShmemInit(); BackendRandomShmemInit(); + BackendSignalFeedbackShmemInit(); #ifdef EXEC_BACKEND diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index f4133953be..e9e41c738b 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -62,6 +62,7 @@ #include "replication/slot.h" #include "replication/walsender.h" #include "rewrite/rewriteHandler.h" +#include "storage/backend_signal.h" #include "storage/bufmgr.h" #include "storage/ipc.h" #include "storage/proc.h" @@ -2923,9 +2924,30 @@ ProcessInterrupts(void) errdetail_recovery_conflict())); } else - ereport(FATAL, - (errcode(ERRCODE_ADMIN_SHUTDOWN), - errmsg("terminating connection due to administrator command"))); + { + if (HasBackendSignalFeedback()) + { + char buffer[MAX_CANCEL_MSG]; + int len; + int sqlerrcode = 0; + + len = ConsumeBackendSignalFeedback(buffer, MAX_CANCEL_MSG, + &sqlerrcode); + if (len == 0) + { + sqlerrcode = ERRCODE_ADMIN_SHUTDOWN; + buffer[0] = '\0'; + } + ereport(FATAL, + (errcode(sqlerrcode), + errmsg("terminating connection due to administrator command: %s%s", + buffer, (len > sizeof(buffer) ? "..." : "")))); + } + else + ereport(FATAL, + (errcode(ERRCODE_ADMIN_SHUTDOWN), + errmsg("terminating connection due to administrator command"))); + } } if (ClientConnectionLost) { @@ -3036,9 +3058,30 @@ ProcessInterrupts(void) if (!DoingCommandRead) { LockErrorCleanup(); - ereport(ERROR, - (errcode(ERRCODE_QUERY_CANCELED), - errmsg("canceling statement due to user request"))); + + if (HasBackendSignalFeedback()) + { + char buffer[MAX_CANCEL_MSG]; + int len; + int sqlerrcode = 0; + + len = ConsumeBackendSignalFeedback(buffer, MAX_CANCEL_MSG, + &sqlerrcode); + if (len == 0) + { + sqlerrcode = ERRCODE_QUERY_CANCELED; + buffer[0] = '\0'; + } + + ereport(ERROR, + (errcode(sqlerrcode), + errmsg("canceling statement due to user request: %s%s", + buffer, (len > sizeof(buffer) ? "..." : "")))); + } + else + ereport(ERROR, + (errcode(ERRCODE_QUERY_CANCELED), + errmsg("canceling statement due to user request"))); } } diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 09e0df290d..65ae347f8d 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -40,6 +40,7 @@ #include "postmaster/autovacuum.h" #include "postmaster/postmaster.h" #include "replication/walsender.h" +#include "storage/backend_signal.h" #include "storage/bufmgr.h" #include "storage/fd.h" #include "storage/ipc.h" @@ -747,6 +748,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, PerformAuthentication(MyProcPort); InitializeSessionUserId(username, useroid); am_superuser = superuser(); + BackendSignalFeedbackInit(MyBackendId); } /* diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 66c6c224a8..354fa302fd 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5973,9 +5973,15 @@ { oid => '2171', descr => 'cancel a server process\' current query', proname => 'pg_cancel_backend', provolatile => 'v', prorettype => 'bool', proargtypes => 'int4', prosrc => 'pg_cancel_backend' }, +{ oid => '3423', descr => 'cancel a server process\' current query and send message', + proname => 'pg_cancel_backend', provolatile => 'v', prorettype => 'bool', + proargtypes => 'int4 text', proisstrict => 'f', prosrc => 'pg_cancel_backend_msg' }, { oid => '2096', descr => 'terminate a server process', proname => 'pg_terminate_backend', provolatile => 'v', prorettype => 'bool', proargtypes => 'int4', prosrc => 'pg_terminate_backend' }, +{ oid => '3424', descr => 'terminate a server process and send message', + proname => 'pg_terminate_backend', provolatile => 'v', prorettype => 'bool', + proargtypes => 'int4 text', proisstrict => 'f', prosrc => 'pg_terminate_backend_msg' }, { oid => '2172', descr => 'prepare for taking an online backup', proname => 'pg_start_backup', provolatile => 'v', proparallel => 'r', prorettype => 'pg_lsn', proargtypes => 'text bool bool', diff --git a/src/include/storage/backend_signal.h b/src/include/storage/backend_signal.h new file mode 100644 index 0000000000..e6431eba9c --- /dev/null +++ b/src/include/storage/backend_signal.h @@ -0,0 +1,27 @@ +/*------------------------------------------------------------------------- + * + * backend_signal.h + * Declarations for backend signalling + * + * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group + * + * src/include/storage/backend_signal.h + * + *------------------------------------------------------------------------- + */ +#ifndef BACKEND_SIGNAL_H +#define BACKEND_SIGNAL_H + +#define MAX_CANCEL_MSG 128 + +extern Size BackendSignalFeedbackShmemSize(void); +extern void BackendSignalFeedbackShmemInit(void); +extern void BackendSignalFeedbackInit(int backend_id); + +extern int SetBackendCancelMessage(pid_t backend, char *message); +extern int SetBackendTerminationMessage(pid_t backend, char *message); +extern int SetBackendSignalFeedback(pid_t backend, char *message, int sqlerrcode); +extern bool HasBackendSignalFeedback(void); +extern int ConsumeBackendSignalFeedback(char *msg, size_t len, int *sqlerrcode); + +#endif /* BACKEND_SIGNAL_H */ diff --git a/src/test/regress/expected/admin_funcs.out b/src/test/regress/expected/admin_funcs.out new file mode 100644 index 0000000000..4661cdbc75 --- /dev/null +++ b/src/test/regress/expected/admin_funcs.out @@ -0,0 +1,29 @@ +select pg_cancel_backend(); +ERROR: function pg_cancel_backend() does not exist +LINE 1: select pg_cancel_backend(); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +select pg_cancel_backend(NULL); + pg_cancel_backend +------------------- + +(1 row) + +select pg_cancel_backend(pg_backend_pid()); +ERROR: canceling statement due to user request +select pg_cancel_backend(NULL, NULL); + pg_cancel_backend +------------------- + +(1 row) + +select pg_cancel_backend(NULL, 'suicide is painless'); + pg_cancel_backend +------------------- + +(1 row) + +select pg_cancel_backend(pg_backend_pid(), 'it brings on many changes'); +ERROR: canceling statement due to user request: it brings on many changes +select pg_cancel_backend(pg_backend_pid(), NULL); +ERROR: canceling statement due to user request diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 16f979c8d9..cdba9235b6 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -84,7 +84,7 @@ test: select_into select_distinct select_distinct_on select_implicit select_havi # ---------- # Another group of parallel tests # ---------- -test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator password func_index +test: brin gin gist spgist privileges init_privs security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator password func_index admin_funcs # ---------- # Another group of parallel tests diff --git a/src/test/regress/sql/admin_funcs.sql b/src/test/regress/sql/admin_funcs.sql new file mode 100644 index 0000000000..420d782b80 --- /dev/null +++ b/src/test/regress/sql/admin_funcs.sql @@ -0,0 +1,7 @@ +select pg_cancel_backend(); +select pg_cancel_backend(NULL); +select pg_cancel_backend(pg_backend_pid()); +select pg_cancel_backend(NULL, NULL); +select pg_cancel_backend(NULL, 'suicide is painless'); +select pg_cancel_backend(pg_backend_pid(), 'it brings on many changes'); +select pg_cancel_backend(pg_backend_pid(), NULL); -- 2.14.1.145.gb3622a4ee