From 70065fb5caeba8592f5de711b4155abbc86f2ed8 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 31 Oct 2018 00:19:10 +0100 Subject: [PATCH 2/2] Allow pg_{cancel|terminate}_backend to pass message This adds an optional message parameter to pg_cancel_backend() and pg_terminate_backend(), whereby the administrator can inform the user why the query was cancelled or connection terminated. A small testcase is added to excercise the new codepath. --- doc/src/sgml/func.sgml | 13 +++++- src/backend/catalog/system_views.sql | 8 ++++ src/backend/storage/ipc/signalfuncs.c | 66 +++++++++++++++++++++++++++---- src/backend/tcop/postgres.c | 61 +++++++++++++++++++++++++--- src/include/catalog/pg_proc.dat | 4 +- src/test/regress/expected/admin_funcs.out | 28 +++++++++++++ src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/admin_funcs.sql | 11 ++++++ 8 files changed, 175 insertions(+), 18 deletions(-) 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 10816f556b..e334e77ae7 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -18756,7 +18756,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 @@ -18781,7 +18781,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 @@ -18812,6 +18812,15 @@ 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, it will + replace the default error message, which in turn will be the error + detail. message is limited to 128 ASCII characters, a + 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: cancellation message text +DETAIL: canceling statement due to user request by process 12345 + diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 53ddc593a8..d164535beb 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1032,6 +1032,14 @@ CREATE OR REPLACE FUNCTION RETURNS boolean STRICT VOLATILE LANGUAGE INTERNAL AS 'pg_promote' PARALLEL RESTRICTED; +CREATE OR REPLACE FUNCTION + pg_cancel_backend(pid int4, message text DEFAULT NULL) + RETURNS bool VOLATILE LANGUAGE internal AS 'pg_cancel_backend' PARALLEL SAFE; + +CREATE OR REPLACE FUNCTION + pg_terminate_backend(pid int4, message text DEFAULT NULL) + RETURNS bool VOLATILE LANGUAGE internal AS 'pg_terminate_backend' PARALLEL SAFE; + -- legacy definition for compatibility with 9.3 CREATE OR REPLACE FUNCTION json_populate_record(base anyelement, from_json json, use_json_as_text boolean DEFAULT false) diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c index 878665b045..f0c02c7008 100644 --- a/src/backend/storage/ipc/signalfuncs.c +++ b/src/backend/storage/ipc/signalfuncs.c @@ -19,9 +19,11 @@ #include "catalog/pg_authid.h" #include "miscadmin.h" #include "postmaster/syslogger.h" +#include "storage/ipc.h" #include "storage/pmsignal.h" #include "storage/proc.h" #include "storage/procarray.h" +#include "storage/signal_message.h" #include "utils/acl.h" #include "utils/builtins.h" @@ -29,9 +31,11 @@ /* * 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 +49,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 +81,30 @@ 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) + { + char *tmp = msg; + + /* + * The message to pass to the signalled backend is currently restricted + * to ASCII only, since the sending backend might use an encoding which + * is incompatible with the receiving with regards to conversion. + */ + while (*tmp != '\0') + { + if (!isascii(*tmp)) + ereport(ERROR, + (errmsg("message is restricted to ASCII only"))); + tmp++; + } + + 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 @@ -110,7 +138,19 @@ pg_signal_backend(int pid, int sig) Datum pg_cancel_backend(PG_FUNCTION_ARGS) { - int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT); + int r; + 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)); + + r = pg_signal_backend(pid, SIGINT, msg); if (r == SIGNAL_BACKEND_NOSUPERUSER) ereport(ERROR, @@ -134,7 +174,19 @@ pg_cancel_backend(PG_FUNCTION_ARGS) Datum pg_terminate_backend(PG_FUNCTION_ARGS) { - int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM); + int r; + 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)); + + r = pg_signal_backend(pid, SIGTERM, msg); if (r == SIGNAL_BACKEND_NOSUPERUSER) ereport(ERROR, @@ -146,7 +198,7 @@ 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); } /* diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index a3b9757565..50c56e0618 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/signal_message.h" #include "storage/bufmgr.h" #include "storage/ipc.h" #include "storage/proc.h" @@ -3013,9 +3014,33 @@ 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; + pid_t pid = 0; + + len = ConsumeBackendSignalFeedback(buffer, MAX_CANCEL_MSG, + &sqlerrcode, &pid, NULL); + if (len == 0) + { + sqlerrcode = ERRCODE_ADMIN_SHUTDOWN; + buffer[0] = '\0'; + } + ereport(FATAL, + (errcode(sqlerrcode), + errmsg("%s%s", + buffer, (len > sizeof(buffer) ? "..." : "")), + errdetail("terminating connection due to administrator command by process %d", + pid))); + } + else + ereport(FATAL, + (errcode(ERRCODE_ADMIN_SHUTDOWN), + errmsg("terminating connection due to administrator command"))); + } } if (ClientConnectionLost) { @@ -3126,9 +3151,33 @@ 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; + pid_t pid = 0; + + len = ConsumeBackendSignalFeedback(buffer, MAX_CANCEL_MSG, + &sqlerrcode, &pid, NULL); + if (len == 0) + { + sqlerrcode = ERRCODE_QUERY_CANCELED; + buffer[0] = '\0'; + } + + ereport(ERROR, + (errcode(sqlerrcode), + errmsg("%s%s", + buffer, (len > sizeof(buffer) ? "..." : "")), + errdetail("canceling statement due to user request by process %d", + pid))); + } + else + ereport(ERROR, + (errcode(ERRCODE_QUERY_CANCELED), + errmsg("canceling statement due to user request"))); } } diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 4026018ba9..11b8830217 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5799,10 +5799,10 @@ { oid => '2171', descr => 'cancel a server process\' current query', proname => 'pg_cancel_backend', provolatile => 'v', prorettype => 'bool', - proargtypes => 'int4', prosrc => 'pg_cancel_backend' }, + proargtypes => 'int4 text', proisstrict => 'f', prosrc => 'pg_cancel_backend' }, { oid => '2096', descr => 'terminate a server process', proname => 'pg_terminate_backend', provolatile => 'v', prorettype => 'bool', - proargtypes => 'int4', prosrc => 'pg_terminate_backend' }, + proargtypes => 'int4 text', proisstrict => 'f', prosrc => 'pg_terminate_backend' }, { 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/test/regress/expected/admin_funcs.out b/src/test/regress/expected/admin_funcs.out new file mode 100644 index 0000000000..84e1835fa6 --- /dev/null +++ b/src/test/regress/expected/admin_funcs.out @@ -0,0 +1,28 @@ +select pg_cancel_backend(NULL); + pg_cancel_backend +------------------- + +(1 row) + +select case + when pg_cancel_backend(pg_backend_pid()) + then pg_sleep(60) +end; +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 case + when pg_cancel_backend(pg_backend_pid(), NULL) + then pg_sleep(60) +end; +ERROR: canceling statement due to user request diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index b5e15501dd..5034cc8c20 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..73a4994535 --- /dev/null +++ b/src/test/regress/sql/admin_funcs.sql @@ -0,0 +1,11 @@ +select pg_cancel_backend(NULL); +select case + when pg_cancel_backend(pg_backend_pid()) + then pg_sleep(60) +end; +select pg_cancel_backend(NULL, NULL); +select pg_cancel_backend(NULL, 'suicide is painless'); +select case + when pg_cancel_backend(pg_backend_pid(), NULL) + then pg_sleep(60) +end; -- 2.14.1.145.gb3622a4ee