Revert addition of pg_terminate_backend() because of race conditions.
authorBruce Momjian
Tue, 15 Apr 2008 20:28:47 +0000 (20:28 +0000)
committerBruce Momjian
Tue, 15 Apr 2008 20:28:47 +0000 (20:28 +0000)
doc/TODO
doc/src/FAQ/TODO.html
doc/src/sgml/func.sgml
doc/src/sgml/runtime.sgml
src/backend/tcop/postgres.c
src/backend/utils/adt/misc.c
src/include/catalog/pg_proc.h
src/include/storage/proc.h
src/include/utils/builtins.h

index c174260ca8531a22f24b2c705ed239e953ba5b6a..36f46113e2123a32a8ea6468c490ea12895f224d 100644 (file)
--- a/doc/TODO
+++ b/doc/TODO
@@ -20,8 +20,17 @@ http://developer.postgresql.org.
 Administration
 ==============
 
-* -Allow administrators to safely terminate individual sessions either
+* Allow administrators to safely terminate individual sessions either
   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
   in-progress when the server terminated abruptly
 
index 59ac9949fe61b89206dedf85658e63cf73cd1f84..6872320c749084a22f07702a730b9fd56a6f863f 100644 (file)
@@ -26,8 +26,16 @@ first.  There is also a developer's wiki at
 

Administration

 
 
    -  
  • -Allow administrators to safely terminate individual sessions either
  • +  
  • Allow administrators to safely terminate individual sessions either
  •    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
  •    in-progress when the server terminated abruptly
     

      http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php

    index 995d1ae9d35b9c3f487af69de83a70157fcaf71a..23a758cd02ed82ccab26a7a1ed5dc66ce8cc17bc 100644 (file)
    @@ -1,4 +1,4 @@
    -
    +
     
      
       Functions and Operators
    @@ -11848,9 +11848,6 @@ SELECT set_config('log_statement_stats', 'off', false);
        
         pg_cancel_backend
        
    -   
    -    pg_terminate_backend
    -   
        
         pg_reload_conf
        
    @@ -11886,13 +11883,6 @@ SELECT set_config('log_statement_stats', 'off', false);
            boolean
            Cancel a backend's current query
           
    -      
    -       
    -        pg_terminate_backend(pid int)
    -        
    -       boolean
    -       Terminate a backend
    -      
           
            
             pg_reload_conf()
    @@ -11917,10 +11907,9 @@ SELECT set_config('log_statement_stats', 'off', false);
        
     
        
    -    pg_cancel_backend and pg_terminate_backend
    -    send a query cancel (SIGINT) signal to a backend process
    -    identified by process ID.  The
    -    process ID of an active backend can be found from
    +    pg_cancel_backend sends a query cancel
    +    (SIGINT) signal to a backend process identified by
    +    process ID.  The process ID of an active backend can be found from
         the procpid column in the
         pg_stat_activity view, or by listing the
         postgres processes on the server with
    index c5222440fc8231cbb01368ea637dfcd852173fba..eb9b937a818246dc7312dbac2c3e04cd151d26ee 100644 (file)
    @@ -1,4 +1,4 @@
    -
    +
     
     
      Operating System Environment
    @@ -1372,13 +1372,6 @@ $ kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`
         well.
        
       
    -
    -  
    -   To terminate a session while allowing other sessions to continue, use
    -   pg_terminate_backend() (
    -   linkend="functions-admin-signal-table">) rather than sending a signal
    -   to the child process.
    -  
      
     
      
    index 60519bd8a95cf2ee86508268b4a9e29c9bc9d93e..d4f84d549b373ec1cfc2867ffb7d2fe91e37b816 100644 (file)
    @@ -8,7 +8,7 @@
      *
      *
      * 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
      *   this is the "main" module of the postgres backend and
    @@ -2541,8 +2541,7 @@ StatementCancelHandler(SIGNAL_ARGS)
             * waiting for input, however.
             */
            if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
    -           CritSectionCount == 0 &&
    -           (!DoingCommandRead || MyProc->terminate))
    +           CritSectionCount == 0 && !DoingCommandRead)
            {
                /* bump holdoff count to make ProcessInterrupts() a no-op */
                /* until we are done getting ready for it */
    @@ -2622,10 +2621,6 @@ ProcessInterrupts(void)
                ereport(ERROR,
                        (errcode(ERRCODE_QUERY_CANCELED),
                         errmsg("canceling autovacuum task")));
    -       else if (MyProc->terminate)
    -           ereport(ERROR,
    -                   (errcode(ERRCODE_ADMIN_SHUTDOWN),
    -                    errmsg("terminating backend due to administrator command")));
            else
                ereport(ERROR,
                        (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 */
            xact_started = false;
     
    -       if (MyProc->terminate)
    -           die(SIGINT);
    -
            /* Now we can allow interrupts again */
            RESUME_INTERRUPTS();
        }
    index d5e794abee24386771aecba93f41c98c93873278..5542f8774491de96ebe24699c4e87b69e7f0dcc4 100644 (file)
    @@ -8,7 +8,7 @@
      *
      *
      * 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 "storage/fd.h"
     #include "storage/pmsignal.h"
    -#include "storage/proc.h"
     #include "storage/procarray.h"
     #include "utils/builtins.h"
     #include "tcop/tcopprot.h"
    @@ -90,7 +89,7 @@ current_query(PG_FUNCTION_ARGS)
      * Functions to send signals to other backends.
      */
     static bool
    -pg_signal_check(int pid)
    +pg_signal_backend(int pid, int sig)
     {
        if (!superuser())
            ereport(ERROR,
    @@ -107,16 +106,7 @@ pg_signal_check(int pid)
                    (errmsg("PID %d is not a PostgreSQL server process", pid)));
            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 */
     #ifdef HAVE_SETSID
        if (kill(-pid, sig))
    @@ -135,43 +125,7 @@ pg_signal_backend(int pid, int sig)
     Datum
     pg_cancel_backend(PG_FUNCTION_ARGS)
     {
    -   int pid = PG_GETARG_INT32(0);
    -   
    -   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);
    +   PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
     }
     
     Datum
    @@ -215,6 +169,17 @@ pg_rotate_logfile(PG_FUNCTION_ARGS)
        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 */
     
     typedef struct
    index 0cfe54959dfa47c5391c80a22dffbdb19ee0bdb6..cb2815ce6edb0d34e8bc06c57f6160c3d41e0cd6 100644 (file)
    @@ -7,7 +7,7 @@
      * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
      * 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
      *   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_ ));
     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_ ));
     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_ ));
    index 8e906a910c1e9f2190df91d65b81f257a0ff8010..d1ab71d0eeb7ea2f5ba71fade79aa1db088a1448 100644 (file)
    @@ -7,7 +7,7 @@
      * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
      * 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        terminate;      /* admin requested termination */
    -
        uint8       vacuumFlags;    /* vacuum-related flags, see above */
     
        /* Info about LWLock the process is currently waiting for, if any. */
    index 629b5b916f2cb1d3d437f91ad43a647459dd0434..8fcdb2c099f711cf49a3cec794a88e5739ec4a74 100644 (file)
    @@ -7,7 +7,7 @@
      * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
      * 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_query(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_tablespace_databases(PG_FUNCTION_ARGS);
     extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);