Revert addition of pg_terminate_backend() because of race conditions.

This commit is contained in:
Bruce Momjian 2008-04-15 20:28:47 +00:00
parent 2b8a795738
commit 76365960d2
9 changed files with 43 additions and 92 deletions

View File

@ -20,8 +20,17 @@ http://developer.postgresql.org.
Administration Administration
============== ==============
* -Allow administrators to safely terminate individual sessions either * Allow administrators to safely terminate individual sessions either
via an SQL function or SIGTERM via an SQL function or SIGTERM
Lock table corruption following SIGTERM of an individual backend
has been reported in 8.0. A possible cause was fixed in 8.1, but
it is unknown whether other problems exist. This item mostly
requires additional testing rather than of writing any new code.
http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php
http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php
* Check for unreferenced table files created by transactions that were * Check for unreferenced table files created by transactions that were
in-progress when the server terminated abruptly in-progress when the server terminated abruptly

View File

@ -26,8 +26,16 @@ first. There is also a developer's wiki at<br/>
<h1><a name="section_2">Administration</a></h1> <h1><a name="section_2">Administration</a></h1>
<ul> <ul>
<li>-<em>Allow administrators to safely terminate individual sessions either</em> <li>Allow administrators to safely terminate individual sessions either
via an SQL function or SIGTERM via an SQL function or SIGTERM
<p> Lock table corruption following SIGTERM of an individual backend
has been reported in 8.0. A possible cause was fixed in 8.1, but
it is unknown whether other problems exist. This item mostly
requires additional testing rather than of writing any new code.
</p>
<p> <a href="http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php">http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php</a>
<a href="http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php">http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php</a>
</p>
</li><li>Check for unreferenced table files created by transactions that were </li><li>Check for unreferenced table files created by transactions that were
in-progress when the server terminated abruptly in-progress when the server terminated abruptly
<p> <a href="http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php">http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php</a> <p> <a href="http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php">http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php</a>

View File

@ -1,4 +1,4 @@
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.431 2008/04/15 13:55:11 momjian Exp $ --> <!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.432 2008/04/15 20:28:46 momjian Exp $ -->
<chapter id="functions"> <chapter id="functions">
<title>Functions and Operators</title> <title>Functions and Operators</title>
@ -11848,9 +11848,6 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm> <indexterm>
<primary>pg_cancel_backend</primary> <primary>pg_cancel_backend</primary>
</indexterm> </indexterm>
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
<indexterm> <indexterm>
<primary>pg_reload_conf</primary> <primary>pg_reload_conf</primary>
</indexterm> </indexterm>
@ -11886,13 +11883,6 @@ SELECT set_config('log_statement_stats', 'off', false);
<entry><type>boolean</type></entry> <entry><type>boolean</type></entry>
<entry>Cancel a backend's current query</entry> <entry>Cancel a backend's current query</entry>
</row> </row>
<row>
<entry>
<literal><function>pg_terminate_backend</function>(<parameter>pid</parameter> <type>int</>)</literal>
</entry>
<entry><type>boolean</type></entry>
<entry>Terminate a backend</entry>
</row>
<row> <row>
<entry> <entry>
<literal><function>pg_reload_conf</function>()</literal> <literal><function>pg_reload_conf</function>()</literal>
@ -11917,10 +11907,9 @@ SELECT set_config('log_statement_stats', 'off', false);
</para> </para>
<para> <para>
<function>pg_cancel_backend</> and <function>pg_terminate_backend</> <function>pg_cancel_backend</> sends a query cancel
send a query cancel (<systemitem>SIGINT</>) signal to a backend process (<systemitem>SIGINT</>) signal to a backend process identified by
identified by process ID. The process ID. The process ID of an active backend can be found from
process ID of an active backend can be found from
the <structfield>procpid</structfield> column in the the <structfield>procpid</structfield> column in the
<structname>pg_stat_activity</structname> view, or by listing the <structname>pg_stat_activity</structname> view, or by listing the
<command>postgres</command> processes on the server with <command>postgres</command> processes on the server with

View File

@ -1,4 +1,4 @@
<!-- $PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.412 2008/04/15 13:55:11 momjian Exp $ --> <!-- $PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.413 2008/04/15 20:28:46 momjian Exp $ -->
<chapter Id="runtime"> <chapter Id="runtime">
<title>Operating System Environment</title> <title>Operating System Environment</title>
@ -1372,13 +1372,6 @@ $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput
well. well.
</para> </para>
</important> </important>
<para>
To terminate a session while allowing other sessions to continue, use
<function>pg_terminate_backend()</> (<xref
linkend="functions-admin-signal-table">) rather than sending a signal
to the child process.
</para>
</sect1> </sect1>
<sect1 id="preventing-server-spoofing"> <sect1 id="preventing-server-spoofing">

View File

@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.549 2008/04/15 13:55:11 momjian Exp $ * $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.550 2008/04/15 20:28:46 momjian Exp $
* *
* NOTES * NOTES
* this is the "main" module of the postgres backend and * this is the "main" module of the postgres backend and
@ -2541,8 +2541,7 @@ StatementCancelHandler(SIGNAL_ARGS)
* waiting for input, however. * waiting for input, however.
*/ */
if (ImmediateInterruptOK && InterruptHoldoffCount == 0 && if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
CritSectionCount == 0 && CritSectionCount == 0 && !DoingCommandRead)
(!DoingCommandRead || MyProc->terminate))
{ {
/* bump holdoff count to make ProcessInterrupts() a no-op */ /* bump holdoff count to make ProcessInterrupts() a no-op */
/* until we are done getting ready for it */ /* until we are done getting ready for it */
@ -2622,10 +2621,6 @@ ProcessInterrupts(void)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_QUERY_CANCELED), (errcode(ERRCODE_QUERY_CANCELED),
errmsg("canceling autovacuum task"))); errmsg("canceling autovacuum task")));
else if (MyProc->terminate)
ereport(ERROR,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
errmsg("terminating backend due to administrator command")));
else else
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_QUERY_CANCELED), (errcode(ERRCODE_QUERY_CANCELED),
@ -3464,9 +3459,6 @@ PostgresMain(int argc, char *argv[], const char *username)
/* We don't have a transaction command open anymore */ /* We don't have a transaction command open anymore */
xact_started = false; xact_started = false;
if (MyProc->terminate)
die(SIGINT);
/* Now we can allow interrupts again */ /* Now we can allow interrupts again */
RESUME_INTERRUPTS(); RESUME_INTERRUPTS();
} }

View File

@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/adt/misc.c,v 1.60 2008/04/15 13:55:11 momjian Exp $ * $PostgreSQL: pgsql/src/backend/utils/adt/misc.c,v 1.61 2008/04/15 20:28:46 momjian Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -27,7 +27,6 @@
#include "postmaster/syslogger.h" #include "postmaster/syslogger.h"
#include "storage/fd.h" #include "storage/fd.h"
#include "storage/pmsignal.h" #include "storage/pmsignal.h"
#include "storage/proc.h"
#include "storage/procarray.h" #include "storage/procarray.h"
#include "utils/builtins.h" #include "utils/builtins.h"
#include "tcop/tcopprot.h" #include "tcop/tcopprot.h"
@ -90,7 +89,7 @@ current_query(PG_FUNCTION_ARGS)
* Functions to send signals to other backends. * Functions to send signals to other backends.
*/ */
static bool static bool
pg_signal_check(int pid) pg_signal_backend(int pid, int sig)
{ {
if (!superuser()) if (!superuser())
ereport(ERROR, ereport(ERROR,
@ -107,16 +106,7 @@ pg_signal_check(int pid)
(errmsg("PID %d is not a PostgreSQL server process", pid))); (errmsg("PID %d is not a PostgreSQL server process", pid)));
return false; return false;
} }
else
return true;
}
/*
* Functions to send signals to other backends.
*/
static bool
pg_signal_backend(int pid, int sig)
{
/* If we have setsid(), signal the backend's whole process group */ /* If we have setsid(), signal the backend's whole process group */
#ifdef HAVE_SETSID #ifdef HAVE_SETSID
if (kill(-pid, sig)) if (kill(-pid, sig))
@ -135,43 +125,7 @@ pg_signal_backend(int pid, int sig)
Datum Datum
pg_cancel_backend(PG_FUNCTION_ARGS) pg_cancel_backend(PG_FUNCTION_ARGS)
{ {
int pid = PG_GETARG_INT32(0); PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
if (pg_signal_check(pid))
PG_RETURN_BOOL(pg_signal_backend(pid, SIGINT));
else
PG_RETURN_BOOL(false);
}
/*
* To cleanly terminate a backend, we set PGPROC(pid)->terminate
* then send a cancel signal. We get ProcArrayLock only when
* setting PGPROC->terminate so the function might fail in
* several places, but that is fine because in those cases the
* backend is already gone.
*/
Datum
pg_terminate_backend(PG_FUNCTION_ARGS)
{
int pid = PG_GETARG_INT32(0);
volatile PGPROC *term_proc;
/* Is this the super-user, and can we find the PGPROC entry for the pid? */
if (pg_signal_check(pid) && (term_proc = BackendPidGetProc(pid)) != NULL)
{
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
/* Recheck now that we have the ProcArray lock. */
if (term_proc->pid == pid)
{
term_proc->terminate = true;
LWLockRelease(ProcArrayLock);
PG_RETURN_BOOL(pg_signal_backend(pid, SIGINT));
}
else
LWLockRelease(ProcArrayLock);
}
PG_RETURN_BOOL(false);
} }
Datum Datum
@ -215,6 +169,17 @@ pg_rotate_logfile(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true); PG_RETURN_BOOL(true);
} }
#ifdef NOT_USED
/* Disabled in 8.0 due to reliability concerns; FIXME someday */
Datum
pg_terminate_backend(PG_FUNCTION_ARGS)
{
PG_RETURN_INT32(pg_signal_backend(PG_GETARG_INT32(0), SIGTERM));
}
#endif
/* Function to find out which databases make use of a tablespace */ /* Function to find out which databases make use of a tablespace */
typedef struct typedef struct

View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.490 2008/04/15 13:55:11 momjian Exp $ * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.491 2008/04/15 20:28:46 momjian Exp $
* *
* NOTES * NOTES
* The script catalog/genbki.sh reads this file and generates .bki * The script catalog/genbki.sh reads this file and generates .bki
@ -3157,8 +3157,6 @@ DESCR("is schema another session's temp schema?");
DATA(insert OID = 2171 ( pg_cancel_backend PGNSP PGUID 12 1 0 f f t f v 1 16 "23" _null_ _null_ _null_ pg_cancel_backend - _null_ _null_ )); DATA(insert OID = 2171 ( pg_cancel_backend PGNSP PGUID 12 1 0 f f t f v 1 16 "23" _null_ _null_ _null_ pg_cancel_backend - _null_ _null_ ));
DESCR("cancel a server process' current query"); DESCR("cancel a server process' current query");
DATA(insert OID = 2096 ( pg_terminate_backend PGNSP PGUID 12 1 0 f f t f v 1 16 "23" _null_ _null_ _null_ pg_terminate_backend - _null_ _null_ ));
DESCR("terminate a server process");
DATA(insert OID = 2172 ( pg_start_backup PGNSP PGUID 12 1 0 f f t f v 1 25 "25" _null_ _null_ _null_ pg_start_backup - _null_ _null_ )); DATA(insert OID = 2172 ( pg_start_backup PGNSP PGUID 12 1 0 f f t f v 1 25 "25" _null_ _null_ _null_ pg_start_backup - _null_ _null_ ));
DESCR("prepare for taking an online backup"); DESCR("prepare for taking an online backup");
DATA(insert OID = 2173 ( pg_stop_backup PGNSP PGUID 12 1 0 f f t f v 0 25 "" _null_ _null_ _null_ pg_stop_backup - _null_ _null_ )); DATA(insert OID = 2173 ( pg_stop_backup PGNSP PGUID 12 1 0 f f t f v 0 25 "" _null_ _null_ _null_ pg_stop_backup - _null_ _null_ ));

View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.105 2008/04/15 13:55:12 momjian Exp $ * $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.106 2008/04/15 20:28:47 momjian Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -91,8 +91,6 @@ struct PGPROC
bool inCommit; /* true if within commit critical section */ bool inCommit; /* true if within commit critical section */
bool terminate; /* admin requested termination */
uint8 vacuumFlags; /* vacuum-related flags, see above */ uint8 vacuumFlags; /* vacuum-related flags, see above */
/* Info about LWLock the process is currently waiting for, if any. */ /* Info about LWLock the process is currently waiting for, if any. */

View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.313 2008/04/15 13:55:12 momjian Exp $ * $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.314 2008/04/15 20:28:47 momjian Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -416,7 +416,6 @@ extern Datum nonnullvalue(PG_FUNCTION_ARGS);
extern Datum current_database(PG_FUNCTION_ARGS); extern Datum current_database(PG_FUNCTION_ARGS);
extern Datum current_query(PG_FUNCTION_ARGS); extern Datum current_query(PG_FUNCTION_ARGS);
extern Datum pg_cancel_backend(PG_FUNCTION_ARGS); extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
extern Datum pg_terminate_backend(PG_FUNCTION_ARGS);
extern Datum pg_reload_conf(PG_FUNCTION_ARGS); extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS); extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS); extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);