Skip to content

Commit 7636596

Browse files
bmomjianbmomjian
committed
Revert addition of pg_terminate_backend() because of race conditions.
1 parent 2b8a795 commit 7636596

File tree

9 files changed

+43
-92
lines changed

9 files changed

+43
-92
lines changed

‎doc/TODO

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,17 @@ http://developer.postgresql.org.
2020
Administration
2121
==============
2222

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

‎doc/src/FAQ/TODO.html

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,16 @@ <h1><a name="section_1">PostgreSQL TODO List</a></h1>
2626
<h1><a name="section_2">Administration</a></h1>
2727

2828
<ul>
29-
<li>-<em>Allow administrators to safely terminate individual sessions either</em>
29+
<li>Allow administrators to safely terminate individual sessions either
3030
via an SQL function or SIGTERM
31+
<p> Lock table corruption following SIGTERM of an individual backend
32+
has been reported in 8.0. A possible cause was fixed in 8.1, but
33+
it is unknown whether other problems exist. This item mostly
34+
requires additional testing rather than of writing any new code.
35+
</p>
36+
<p> <a href="http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php">http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php</a>
37+
<a href="http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php">http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php</a>
38+
</p>
3139
</li><li>Check for unreferenced table files created by transactions that were
3240
in-progress when the server terminated abruptly
3341
<p> <a href="http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php">http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php</a>

‎doc/src/sgml/func.sgml

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.431 2008/04/15 13:55:11 momjian Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.432 2008/04/15 20:28:46 momjian Exp $ -->
22

33
<chapter id="functions">
44
<title>Functions and Operators</title>
@@ -11848,9 +11848,6 @@ SELECT set_config('log_statement_stats', 'off', false);
1184811848
<indexterm>
1184911849
<primary>pg_cancel_backend</primary>
1185011850
</indexterm>
11851-
<indexterm>
11852-
<primary>pg_terminate_backend</primary>
11853-
</indexterm>
1185411851
<indexterm>
1185511852
<primary>pg_reload_conf</primary>
1185611853
</indexterm>
@@ -11886,13 +11883,6 @@ SELECT set_config('log_statement_stats', 'off', false);
1188611883
<entry><type>boolean</type></entry>
1188711884
<entry>Cancel a backend's current query</entry>
1188811885
</row>
11889-
<row>
11890-
<entry>
11891-
<literal><function>pg_terminate_backend</function>(<parameter>pid</parameter> <type>int</>)</literal>
11892-
</entry>
11893-
<entry><type>boolean</type></entry>
11894-
<entry>Terminate a backend</entry>
11895-
</row>
1189611886
<row>
1189711887
<entry>
1189811888
<literal><function>pg_reload_conf</function>()</literal>
@@ -11917,10 +11907,9 @@ SELECT set_config('log_statement_stats', 'off', false);
1191711907
</para>
1191811908

1191911909
<para>
11920-
<function>pg_cancel_backend</> and <function>pg_terminate_backend</>
11921-
send a query cancel (<systemitem>SIGINT</>) signal to a backend process
11922-
identified by process ID. The
11923-
process ID of an active backend can be found from
11910+
<function>pg_cancel_backend</> sends a query cancel
11911+
(<systemitem>SIGINT</>) signal to a backend process identified by
11912+
process ID. The process ID of an active backend can be found from
1192411913
the <structfield>procpid</structfield> column in the
1192511914
<structname>pg_stat_activity</structname> view, or by listing the
1192611915
<command>postgres</command> processes on the server with

‎doc/src/sgml/runtime.sgml

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.412 2008/04/15 13:55:11 momjian Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.413 2008/04/15 20:28:46 momjian Exp $ -->
22

33
<chapter Id="runtime">
44
<title>Operating System Environment</title>
@@ -1372,13 +1372,6 @@ $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput
13721372
well.
13731373
</para>
13741374
</important>
1375-
1376-
<para>
1377-
To terminate a session while allowing other sessions to continue, use
1378-
<function>pg_terminate_backend()</> (<xref
1379-
linkend="functions-admin-signal-table">) rather than sending a signal
1380-
to the child process.
1381-
</para>
13821375
</sect1>
13831376

13841377
<sect1 id="preventing-server-spoofing">

‎src/backend/tcop/postgres.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.549 2008/04/15 13:55:11 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.550 2008/04/15 20:28:46 momjian Exp $
1212
*
1313
* NOTES
1414
* this is the "main" module of the postgres backend and
@@ -2541,8 +2541,7 @@ StatementCancelHandler(SIGNAL_ARGS)
25412541
* waiting for input, however.
25422542
*/
25432543
if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
2544-
CritSectionCount == 0 &&
2545-
(!DoingCommandRead || MyProc->terminate))
2544+
CritSectionCount == 0 && !DoingCommandRead)
25462545
{
25472546
/* bump holdoff count to make ProcessInterrupts() a no-op */
25482547
/* until we are done getting ready for it */
@@ -2622,10 +2621,6 @@ ProcessInterrupts(void)
26222621
ereport(ERROR,
26232622
(errcode(ERRCODE_QUERY_CANCELED),
26242623
errmsg("canceling autovacuum task")));
2625-
else if (MyProc->terminate)
2626-
ereport(ERROR,
2627-
(errcode(ERRCODE_ADMIN_SHUTDOWN),
2628-
errmsg("terminating backend due to administrator command")));
26292624
else
26302625
ereport(ERROR,
26312626
(errcode(ERRCODE_QUERY_CANCELED),
@@ -3464,9 +3459,6 @@ PostgresMain(int argc, char *argv[], const char *username)
34643459
/* We don't have a transaction command open anymore */
34653460
xact_started = false;
34663461

3467-
if (MyProc->terminate)
3468-
die(SIGINT);
3469-
34703462
/* Now we can allow interrupts again */
34713463
RESUME_INTERRUPTS();
34723464
}

‎src/backend/utils/adt/misc.c

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/misc.c,v 1.60 2008/04/15 13:55:11 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/misc.c,v 1.61 2008/04/15 20:28:46 momjian Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -27,7 +27,6 @@
2727
#include "postmaster/syslogger.h"
2828
#include "storage/fd.h"
2929
#include "storage/pmsignal.h"
30-
#include "storage/proc.h"
3130
#include "storage/procarray.h"
3231
#include "utils/builtins.h"
3332
#include "tcop/tcopprot.h"
@@ -90,7 +89,7 @@ current_query(PG_FUNCTION_ARGS)
9089
* Functions to send signals to other backends.
9190
*/
9291
static bool
93-
pg_signal_check(int pid)
92+
pg_signal_backend(int pid, int sig)
9493
{
9594
if (!superuser())
9695
ereport(ERROR,
@@ -107,16 +106,7 @@ pg_signal_check(int pid)
107106
(errmsg("PID %d is not a PostgreSQL server process", pid)));
108107
return false;
109108
}
110-
else
111-
return true;
112-
}
113109

114-
/*
115-
* Functions to send signals to other backends.
116-
*/
117-
static bool
118-
pg_signal_backend(int pid, int sig)
119-
{
120110
/* If we have setsid(), signal the backend's whole process group */
121111
#ifdef HAVE_SETSID
122112
if (kill(-pid, sig))
@@ -135,43 +125,7 @@ pg_signal_backend(int pid, int sig)
135125
Datum
136126
pg_cancel_backend(PG_FUNCTION_ARGS)
137127
{
138-
int pid = PG_GETARG_INT32(0);
139-
140-
if (pg_signal_check(pid))
141-
PG_RETURN_BOOL(pg_signal_backend(pid, SIGINT));
142-
else
143-
PG_RETURN_BOOL(false);
144-
}
145-
146-
/*
147-
* To cleanly terminate a backend, we set PGPROC(pid)->terminate
148-
* then send a cancel signal. We get ProcArrayLock only when
149-
* setting PGPROC->terminate so the function might fail in
150-
* several places, but that is fine because in those cases the
151-
* backend is already gone.
152-
*/
153-
Datum
154-
pg_terminate_backend(PG_FUNCTION_ARGS)
155-
{
156-
int pid = PG_GETARG_INT32(0);
157-
volatile PGPROC *term_proc;
158-
159-
/* Is this the super-user, and can we find the PGPROC entry for the pid? */
160-
if (pg_signal_check(pid) && (term_proc = BackendPidGetProc(pid)) != NULL)
161-
{
162-
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
163-
/* Recheck now that we have the ProcArray lock. */
164-
if (term_proc->pid == pid)
165-
{
166-
term_proc->terminate = true;
167-
LWLockRelease(ProcArrayLock);
168-
PG_RETURN_BOOL(pg_signal_backend(pid, SIGINT));
169-
}
170-
else
171-
LWLockRelease(ProcArrayLock);
172-
}
173-
174-
PG_RETURN_BOOL(false);
128+
PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
175129
}
176130

177131
Datum
@@ -215,6 +169,17 @@ pg_rotate_logfile(PG_FUNCTION_ARGS)
215169
PG_RETURN_BOOL(true);
216170
}
217171

172+
#ifdef NOT_USED
173+
174+
/* Disabled in 8.0 due to reliability concerns; FIXME someday */
175+
Datum
176+
pg_terminate_backend(PG_FUNCTION_ARGS)
177+
{
178+
PG_RETURN_INT32(pg_signal_backend(PG_GETARG_INT32(0), SIGTERM));
179+
}
180+
#endif
181+
182+
218183
/* Function to find out which databases make use of a tablespace */
219184

220185
typedef struct

‎src/include/catalog/pg_proc.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.490 2008/04/15 13:55:11 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.491 2008/04/15 20:28:46 momjian Exp $
1111
*
1212
* NOTES
1313
* The script catalog/genbki.sh reads this file and generates .bki
@@ -3157,8 +3157,6 @@ DESCR("is schema another session's temp schema?");
31573157

31583158
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_ ));
31593159
DESCR("cancel a server process' current query");
3160-
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_ ));
3161-
DESCR("terminate a server process");
31623160
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_ ));
31633161
DESCR("prepare for taking an online backup");
31643162
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_ ));

‎src/include/storage/proc.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.105 2008/04/15 13:55:12 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.106 2008/04/15 20:28:47 momjian Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -91,8 +91,6 @@ struct PGPROC
9191

9292
bool inCommit; /* true if within commit critical section */
9393

94-
bool terminate; /* admin requested termination */
95-
9694
uint8 vacuumFlags; /* vacuum-related flags, see above */
9795

9896
/* Info about LWLock the process is currently waiting for, if any. */

‎src/include/utils/builtins.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.313 2008/04/15 13:55:12 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/utils/builtins.h,v 1.314 2008/04/15 20:28:47 momjian Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -416,7 +416,6 @@ extern Datum nonnullvalue(PG_FUNCTION_ARGS);
416416
extern Datum current_database(PG_FUNCTION_ARGS);
417417
extern Datum current_query(PG_FUNCTION_ARGS);
418418
extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
419-
extern Datum pg_terminate_backend(PG_FUNCTION_ARGS);
420419
extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
421420
extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
422421
extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);

0 commit comments

Comments
 (0)