Commit 55e4ef13 authored by Tom Lane's avatar Tom Lane

Code review for statement_timeout patch. Fix some race conditions

between signal handler and enable/disable code, avoid accumulation of
timing error due to trying to maintain remaining-time instead of
absolute-end-time, disable timeout before commit not after.
parent 8a45a2e9
$Header: /cvsroot/pgsql/src/backend/storage/lmgr/README,v 1.11 2002/07/19 00:17:40 momjian Exp $
$Header: /cvsroot/pgsql/src/backend/storage/lmgr/README,v 1.12 2002/10/31 21:34:16 tgl Exp $
LOCKING OVERVIEW
......@@ -392,7 +392,7 @@ Miscellaneous notes:
asynchronous invocation of deadlock checking. A deadlock cycle in the WFG
is formed when the last edge in the cycle is added; therefore the last
process in the cycle to wait (the one from which that edge is outgoing) is
certain to detect and resolve the cycle when it later runs HandleDeadLock.
certain to detect and resolve the cycle when it later runs CheckDeadLock.
This holds even if that edge addition created multiple cycles; the process
may indeed abort without ever noticing those additional cycles, but we
don't particularly care. The only other possible creation of deadlocks is
......@@ -402,7 +402,7 @@ it attempts to actually execute any rearrangement.
2. It is not certain that a deadlock will be resolved by aborting the
last-to-wait process. If earlier waiters in the cycle have not yet run
HandleDeadLock, then the first one to do so will be the victim.
CheckDeadLock, then the first one to do so will be the victim.
3. No live (wakable) process can be missed by ProcLockWakeup, since it
examines every member of the wait queue (this was not true in the 7.0
......
......@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/lock.c,v 1.116 2002/09/26 05:18:30 momjian Exp $
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/lock.c,v 1.117 2002/10/31 21:34:16 tgl Exp $
*
* NOTES
* Outside modules can create a lock table and acquire/release
......@@ -882,7 +882,7 @@ WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode,
/*
* NOTE: Think not to put any shared-state cleanup after the call to
* ProcSleep, in either the normal or failure path. The lock state
* must be fully set by the lock grantor, or by HandleDeadLock if we
* must be fully set by the lock grantor, or by CheckDeadLock if we
* give up waiting for the lock. This is necessary because of the
* possibility that a cancel/die interrupt will interrupt ProcSleep
* after someone else grants us the lock, but before we've noticed it.
......@@ -899,7 +899,7 @@ WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode,
holder) != STATUS_OK)
{
/*
* We failed as a result of a deadlock, see HandleDeadLock(). Quit
* We failed as a result of a deadlock, see CheckDeadLock(). Quit
* now. Removal of the holder and lock objects, if no longer
* needed, will happen in xact cleanup (see above for motivation).
*/
......
This diff is collapsed.
......@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.306 2002/10/24 23:19:13 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.307 2002/10/31 21:34:16 tgl Exp $
*
* NOTES
* this is the "main" module of the postgres backend and
......@@ -75,8 +75,6 @@ char *debug_query_string; /* for pgmonitor and
/* Note: whereToSendOutput is initialized for the bootstrap/standalone case */
CommandDest whereToSendOutput = Debug;
extern int StatementTimeout;
static bool dontExecute = false;
/* note: these declarations had better match tcopprot.h */
......@@ -582,9 +580,6 @@ pg_exec_query_string(StringInfo query_string, /* string to execute */
start_xact_command();
xact_started = true;
if (StatementTimeout)
enable_sig_alarm(StatementTimeout, true);
/*
* parse_context *must* be different from the execution memory
* context, else the context reset at the bottom of the loop will
......@@ -931,8 +926,6 @@ pg_exec_query_string(StringInfo query_string, /* string to execute */
EndCommand(commandTag, dest);
} /* end loop over parsetrees */
disable_sig_alarm(true);
/*
* Close down transaction statement, if one is open. (Note that this
* will only happen if the querystring was empty.)
......@@ -964,6 +957,10 @@ start_xact_command(void)
{
elog(DEBUG1, "StartTransactionCommand");
StartTransactionCommand(false);
/* Set statement timeout running, if any */
if (StatementTimeout > 0)
enable_sig_alarm(StatementTimeout, true);
}
static void
......@@ -972,6 +969,9 @@ finish_xact_command(bool forceCommit)
/* Invoke IMMEDIATE constraint triggers */
DeferredTriggerEndQuery();
/* Cancel any active statement timeout before committing */
disable_sig_alarm(true);
/* Now commit the command */
elog(DEBUG1, "CommitTransactionCommand");
......@@ -1047,7 +1047,7 @@ die(SIGNAL_ARGS)
/* until we are done getting ready for it */
InterruptHoldoffCount++;
DisableNotifyInterrupt();
/* Make sure HandleDeadLock won't run while shutting down... */
/* Make sure CheckDeadLock won't run while shutting down... */
LockWaitCancel();
InterruptHoldoffCount--;
ProcessInterrupts();
......@@ -1648,8 +1648,7 @@ PostgresMain(int argc, char *argv[], const char *username)
pqsignal(SIGINT, StatementCancelHandler); /* cancel current query */
pqsignal(SIGTERM, die); /* cancel current query and exit */
pqsignal(SIGQUIT, quickdie); /* hard crash time */
pqsignal(SIGALRM, handle_sig_alarm); /* check for deadlock
* after timeout */
pqsignal(SIGALRM, handle_sig_alarm); /* timeout conditions */
/*
* Ignore failure to write to frontend. Note: if frontend closes
......@@ -1782,7 +1781,7 @@ PostgresMain(int argc, char *argv[], const char *username)
if (!IsUnderPostmaster)
{
puts("\nPOSTGRES backend interactive interface ");
puts("$Revision: 1.306 $ $Date: 2002/10/24 23:19:13 $\n");
puts("$Revision: 1.307 $ $Date: 2002/10/31 21:34:16 $\n");
}
/*
......@@ -1829,6 +1828,8 @@ PostgresMain(int argc, char *argv[], const char *username)
QueryCancelPending = false;
InterruptHoldoffCount = 1;
CritSectionCount = 0; /* should be unnecessary, but... */
disable_sig_alarm(true);
QueryCancelPending = false; /* again in case timeout occurred */
DisableNotifyInterrupt();
debug_query_string = NULL;
......@@ -1915,9 +1916,6 @@ PostgresMain(int argc, char *argv[], const char *username)
QueryCancelPending = false; /* forget any earlier CANCEL
* signal */
/* Stop any statement timer */
disable_sig_alarm(true);
EnableNotifyInterrupt();
/* Allow "die" interrupt to be processed while waiting */
......
......@@ -5,7 +5,7 @@
* command, configuration file, and command line options.
* See src/backend/utils/misc/README for more information.
*
* $Header: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v 1.97 2002/10/02 16:27:57 momjian Exp $
* $Header: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v 1.98 2002/10/31 21:34:17 tgl Exp $
*
* Copyright 2000 by PostgreSQL Global Development Group
* Written by Peter Eisentraut <peter_e@gmx.net>.
......@@ -56,7 +56,6 @@
extern bool Log_connections;
extern int PreAuthDelay;
extern int AuthenticationTimeout;
extern int StatementTimeout;
extern int CheckPointTimeout;
extern bool autocommit;
extern int CommitDelay;
......
......@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $Id: proc.h,v 1.61 2002/10/21 18:57:34 petere Exp $
* $Id: proc.h,v 1.62 2002/10/31 21:34:17 tgl Exp $
*
*-------------------------------------------------------------------------
*/
......@@ -86,8 +86,9 @@ typedef struct PROC_HDR
} PROC_HDR;
/* configurable option */
/* configurable options */
extern int DeadlockTimeout;
extern int StatementTimeout;
/*
......@@ -105,7 +106,6 @@ extern int ProcSleep(LOCKMETHODTABLE *lockMethodTable, LOCKMODE lockmode,
extern PGPROC *ProcWakeup(PGPROC *proc, int errType);
extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock);
extern bool LockWaitCancel(void);
extern void CheckDeadLock(void);
extern void ProcWaitForSignal(void);
extern void ProcCancelWaitForSignal(void);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment