2013-03-15 18:53 keltezéssel, Tom Lane írta:
Boszormenyi Zoltan<z...@cybertec.at> writes:
[ 2-lock_timeout-v33.patch ]
I looked at this patch a bit. I don't understand why you've chosen to
alter the API of the enable_timeout variants to have a bool result that
says "I didn't bother to process the timeout because it would have fired
immediately".
I don't know either...
That is not an advantage for any call site that I can
see: it just means that the caller needs an extra, very difficult to
test code path to handle what would normally be handled by a timeout
interrupt. Even if it were a good API choice, you've broken a number of
existing call sites that the patch fails to touch (eg in postmaster.c
and postgres.c). It's not acceptable to define a failure return from
enable_timeout_after and then let callers assume that the failure can't
happen.
Please undo that.
Done.
Also, I'm not really enamored of the choice to use List* infrastructure
for enable_timeouts(). That doesn't appear to be especially convenient
for any caller, and what it does do is create memory-leak risks for most
of them (if they forget to free the list cells, perhaps as a result of
an error exit). I think a local-variable array of TimeoutParams[]
structs would serve better for most use-cases.
Changed. However, the first member of the structure is
"TimeoutId id" and a sensible end-of-array value can be -1.
Some versions of GCC (maybe other compilers, too) complain
if a constant is assigned to an enum which is outside of the
declared values of the enum. So I added a "NO_TIMEOUT = -1"
to enum TimeoutId. Comments?
Another thought is that the PGSemaphoreTimedLock implementations all
share the same bug, which is that if the "condition" callback returns
true immediately after we acquire the semaphore, they will all wrongly
return false indicating that the semaphore wasn't acquired.
You are right. Fixed.
(BTW,
I do not like either the variable name "condition" or the typedef name
"TimeoutCondition" for something that's actually a callback function
pointer.)
How about "TimeoutCallback" and "callback_fn"?
In the same vein, this comment change:
* 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 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. Hence, after granting,
! * the locktable state must fully reflect the fact that we own the lock;
! * we can't do additional work on return.
*
* We can and do use a PG_TRY block to try to clean up after failure, but
* this still has a major limitation: elog(FATAL) can occur while waiting
--- 1594,1606 ----
/*
* 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 CheckDeadLock or by ProcSleep
! * itself in case a timeout is detected or 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. Hence, after granting, the locktable state
must
! * fully reflect the fact that we own the lock; we can't do additional
work
! * on return.
*
suggests that you've failed to grasp the fundamental race-condition
problem here. ProcSleep can't do cleanup after the fact any more than
its callers can, because otherwise it has a race condition with some
other process deciding to grant it the lock at about the same time its
timeout goes off. I think the changes in ProcSleep that alter the
state-cleanup behavior are just plain wrong, and what you need to do
is make that look more like the existing mechanisms that clean up when
statement timeout occurs.
This was the most enlightening comment up to now.
It seems no one else understood the timer code but you...
Thanks very much.
I hope the way I did it is right. I factored out the core of
StatementCancelHandler() into a common function that can
be used by the lock_timeout interrupt as its timeout callback
function. Now the code doesn't need PGSemaphoreTimedLock().
While I was thinking about how this thing works, I recognized
that I also need to set the timeout indicator to false after checking
it in ProcessInterrupts().
The reason is that it's needed is this scenario:
1. lock_timeout is set and the transaction throws its error
2. lock_timeout is unset before the next transaction
3. the user presses Ctrl-C during the next transaction
In this case, the second transaction would report the
lock timeout error, since the indicator was still set.
The other way would be to call disable_all_timeouts(false)
unconditionally in ProcessInterrupts() but setting only the indicator
is faster and it doesn't interfere with unknown registered timeout.
Also, the reason is that it's disable_timeout_indicator() instead of
a generic set_timeout_indicator() is that the generic one may be
abused to produce false positives.
Best regards,
Zoltán Böszörményi
regards, tom lane
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web:http://www.postgresql-support.de
http://www.postgresql.at/
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ae6ee60..c554ab1 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5080,7 +5080,10 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
milliseconds, starting from the time the command arrives at the server
from the client. If <varname>log_min_error_statement</> is set to
<literal>ERROR</> or lower, the statement that timed out will also be
- logged. A value of zero (the default) turns this off.
+ logged. The timeout may happen any time, i.e. while waiting for locks
+ on database objects or in case of a large result set, during data
+ retrieval from the server after all locks were successfully acquired.
+ A value of zero (the default) turns this off.
</para>
<para>
@@ -5091,6 +5094,33 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</listitem>
</varlistentry>
+ <varlistentry id="guc-lock-timeout" xreflabel="lock_timeout">
+ <term><varname>lock_timeout</varname> (<type>integer</type>)</term>
+ <indexterm>
+ <primary><varname>lock_timeout</> configuration parameter</primary>
+ </indexterm>
+ <listitem>
+ <para>
+ Abort any statement which waits longer than the specified number of
+ milliseconds while attempting to acquire a lock on rows, pages,
+ tables, indices or other objects. The timeout applies to each of
+ the locks individually.
+ </para>
+ <para>
+ As opposed to <varname>statement_timeout</>, this timeout (and the error)
+ may only occur while waiting for locks. If <varname>log_min_error_statement</>
+ is set to <literal>ERROR</> or lower, the statement that timed out will
+ also be logged. A value of zero (the default) turns this off.
+ </para>
+
+ <para>
+ Setting <varname>lock_timeout</> in
+ <filename>postgresql.conf</> is not recommended because it
+ affects all sessions.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-vacuum-freeze-table-age" xreflabel="vacuum_freeze_table_age">
<term><varname>vacuum_freeze_table_age</varname> (<type>integer</type>)</term>
<indexterm>
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index 05acbc4..3de08d5 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -39,10 +39,12 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="PARAMETER">name</replaceable> [ * ]
<literal>NOWAIT</literal> is specified, <command>LOCK
TABLE</command> does not wait to acquire the desired lock: if it
cannot be acquired immediately, the command is aborted and an
- error is emitted. Once obtained, the lock is held for the
- remainder of the current transaction. (There is no <command>UNLOCK
- TABLE</command> command; locks are always released at transaction
- end.)
+ error is emitted. If <varname>lock_timeout</varname> is set and
+ the lock cannot be acquired under the specified timeout,
+ the command is aborted and an error is emitted. Once obtained,
+ the lock is held for the remainder of the current transaction.
+ (There is no <command>UNLOCK TABLE</command> command; locks are
+ always released at transaction end.)
</para>
<para>
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 1d3f854..e7106b2 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -1301,6 +1301,14 @@ KEY SHARE
</para>
<para>
+ If <literal>NOWAIT</> option is not specified and either
+ <varname>statement_timeout</varname> or <varname>lock_timeout</varname>
+ is set and the lock or statement needs to wait more than the specified
+ timeout, the command reports an error after timing out, rather than
+ waiting until the lock becomes available.
+ </para>
+
+ <para>
If specific tables are named in a locking clause,
then only rows coming from those tables are locked; any other
tables used in the <command>SELECT</command> are simply read as
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index a903f12..90e67ca 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -424,12 +424,19 @@ ResolveRecoveryConflictWithBufferPin(void)
}
else
{
+ TimeoutParams params[3] = {
+ { STANDBY_DEADLOCK_TIMEOUT, TMPARAM_AFTER },
+ { STANDBY_TIMEOUT, TMPARAM_AT },
+ { NO_TIMEOUT }
+ };
+
/*
* Wake up at ltime, and check for deadlocks as well if we will be
* waiting longer than deadlock_timeout
*/
- enable_timeout_after(STANDBY_DEADLOCK_TIMEOUT, DeadlockTimeout);
- enable_timeout_at(STANDBY_TIMEOUT, ltime);
+ params[0].delay_ms = DeadlockTimeout;
+ params[1].fin_time = ltime;
+ enable_timeouts(params);
}
/* Wait to be signaled by UnpinBuffer() */
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 2e012fa..1b6253c 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -55,6 +55,7 @@
/* GUC variables */
int DeadlockTimeout = 1000;
int StatementTimeout = 0;
+int LockTimeout = 0;
bool log_lock_waits = false;
/* Pointer to this process's PGPROC and PGXACT structs, if any */
@@ -665,6 +666,12 @@ void
LockErrorCleanup(void)
{
LWLockId partitionLock;
+ TimeoutParams timeouts[3] =
+ {
+ { DEADLOCK_TIMEOUT },
+ { LOCK_TIMEOUT, .keep_indicator = true },
+ { NO_TIMEOUT }
+ };
AbortStrongLockAcquire();
@@ -672,8 +679,11 @@ LockErrorCleanup(void)
if (lockAwaited == NULL)
return;
- /* Turn off the deadlock timer, if it's still running (see ProcSleep) */
- disable_timeout(DEADLOCK_TIMEOUT, false);
+ /*
+ * Turn off the deadlock and lock timeout timers,
+ * if they are still running (see ProcSleep).
+ */
+ disable_timeouts(timeouts);
/* Unlink myself from the wait queue, if on it (might not be anymore!) */
partitionLock = LockHashPartitionLock(lockAwaited->hashcode);
@@ -936,6 +946,11 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
LOCKMASK myHeldLocks = MyProc->heldLocks;
bool early_deadlock = false;
bool allow_autovacuum_cancel = true;
+ TimeoutParams timeouts[3] = {
+ { DEADLOCK_TIMEOUT, TMPARAM_AFTER },
+ { LOCK_TIMEOUT, TMPARAM_AFTER },
+ { NO_TIMEOUT }
+ };
int myWaitStatus;
PGPROC *proc;
int i;
@@ -1065,15 +1080,26 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
deadlock_state = DS_NOT_YET_CHECKED;
/*
- * Set timer so we can wake up after awhile and check for a deadlock. If a
- * deadlock is detected, the handler releases the process's semaphore and
- * sets MyProc->waitStatus = STATUS_ERROR, allowing us to know that we
- * must report failure rather than success.
+ * Set timers so:
+ * a) we can wake up after awhile and check for a deadlock. If a
+ * deadlock is detected, the handler releases the process's semaphore and
+ * sets MyProc->waitStatus = STATUS_ERROR, allowing us to know that we
+ * must report failure rather than success.
+ * b) detect lock timeout.
*
* By delaying the check until we've waited for a bit, we can avoid
* running the rather expensive deadlock-check code in most cases.
+ *
+ * If both timeout sources are set, then enable them all once.
*/
- enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
+ timeouts[0].delay_ms = DeadlockTimeout;
+
+ if (LockTimeout > 0)
+ timeouts[1].delay_ms = LockTimeout;
+ else
+ timeouts[1].id = NO_TIMEOUT;
+
+ enable_timeouts(timeouts);
/*
* If someone wakes us between LWLockRelease and PGSemaphoreLock,
@@ -1240,9 +1266,9 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
} while (myWaitStatus == STATUS_WAITING);
/*
- * Disable the timer, if it's still running
+ * Disable the timers, if they are still running
*/
- disable_timeout(DEADLOCK_TIMEOUT, false);
+ disable_timeouts(timeouts);
/*
* Re-acquire the lock table's partition lock. We have to do this to hold
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 407c548..b938edf 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2609,14 +2609,13 @@ die(SIGNAL_ARGS)
}
/*
- * Query-cancel signal from postmaster: abort current transaction
- * at soonest convenient time
+ * ProcessStatementCancel
+ *
+ * Common processing between STATEMENT_TIMEOUT and LOCK_TIMEOUT
*/
void
-StatementCancelHandler(SIGNAL_ARGS)
+ProcessStatementCancel(void)
{
- int save_errno = errno;
-
/*
* Don't joggle the elbow of proc_exit
*/
@@ -2642,6 +2641,17 @@ StatementCancelHandler(SIGNAL_ARGS)
ProcessInterrupts();
}
}
+}
+/*
+ * Query-cancel signal from postmaster: abort current transaction
+ * at soonest convenient time
+ */
+void
+StatementCancelHandler(SIGNAL_ARGS)
+{
+ int save_errno = errno;
+
+ ProcessStatementCancel();
/* If we're still here, waken anything waiting on the process latch */
if (MyProc)
@@ -2885,6 +2895,7 @@ ProcessInterrupts(void)
}
if (get_timeout_indicator(STATEMENT_TIMEOUT))
{
+ disable_timeout_indicator(STATEMENT_TIMEOUT);
ImmediateInterruptOK = false; /* not idle anymore */
DisableNotifyInterrupt();
DisableCatchupInterrupt();
@@ -2892,6 +2903,16 @@ ProcessInterrupts(void)
(errcode(ERRCODE_QUERY_CANCELED),
errmsg("canceling statement due to statement timeout")));
}
+ if (get_timeout_indicator(LOCK_TIMEOUT))
+ {
+ disable_timeout_indicator(LOCK_TIMEOUT);
+ ImmediateInterruptOK = false; /* not idle anymore */
+ DisableNotifyInterrupt();
+ DisableCatchupInterrupt();
+ ereport(ERROR,
+ (errcode(ERRCODE_QUERY_CANCELED),
+ errmsg("canceling statement due to lock timeout")));
+ }
if (IsAutoVacuumWorkerProcess())
{
ImmediateInterruptOK = false; /* not idle anymore */
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 8427006..cb28169 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -67,6 +67,7 @@ static void CheckMyDatabase(const char *name, bool am_superuser);
static void InitCommunication(void);
static void ShutdownPostgres(int code, Datum arg);
static void StatementTimeoutHandler(void);
+static void LockTimeoutHandler(void);
static bool ThereIsAtLeastOneRole(void);
static void process_startup_options(Port *port, bool am_superuser);
static void process_settings(Oid databaseid, Oid roleid);
@@ -535,6 +536,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
{
RegisterTimeout(DEADLOCK_TIMEOUT, CheckDeadLock);
RegisterTimeout(STATEMENT_TIMEOUT, StatementTimeoutHandler);
+ RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler);
}
/*
@@ -1054,6 +1056,22 @@ StatementTimeoutHandler(void)
/*
+ * LOCK_TIMEOUT handler
+ *
+ * It's doing the same processing as statement timeout
+ * does from SIGINT context.
+ */
+static void
+LockTimeoutHandler(void)
+{
+ int save_errno = errno;
+
+ ProcessStatementCancel();
+
+ errno = save_errno;
+}
+
+/*
* Returns true if at least one role is defined in this database cluster.
*/
static bool
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 98149fc..03b1e17 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1862,6 +1862,17 @@ static struct config_int ConfigureNamesInt[] =
},
{
+ {"lock_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the maximum allowed timeout for any lock taken by a statement."),
+ gettext_noop("A value of 0 turns off the timeout."),
+ GUC_UNIT_MS
+ },
+ &LockTimeout,
+ 0, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
+ {
{"vacuum_freeze_min_age", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Minimum age at which VACUUM should freeze a table row."),
NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 62aea2f..01a99cf 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -532,6 +532,7 @@
#------------------------------------------------------------------------------
#deadlock_timeout = 1s
+#lock_timeout = 0
#max_locks_per_transaction = 64 # min 10
# (change requires restart)
# Note: Each lock table slot uses ~270 bytes of shared memory, and there are
diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c
index 944c9a7..db634f7 100644
--- a/src/backend/utils/misc/timeout.c
+++ b/src/backend/utils/misc/timeout.c
@@ -153,6 +153,30 @@ schedule_alarm(TimestampTz now)
}
}
+/*
+ * Disable alarm
+ *
+ * If num_active_timeouts is zero, we don't have to call setitimer. There
+ * should not be any pending interrupt, and even if there is, the worst
+ * possible case is that the signal handler fires during schedule_alarm.
+ * (If it fires at any point before insert_timeout has incremented
+ * num_active_timeouts, it will do nothing.) In that case we could end up
+ * scheduling a useless interrupt ... but when the interrupt does happen,
+ * the signal handler will do nothing, so it's all good.
+ */
+static void
+disable_alarm(void)
+{
+ struct itimerval timeval;
+
+ if (num_active_timeouts > 0)
+ {
+ MemSet(&timeval, 0, sizeof(struct itimerval));
+ if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
+ elog(FATAL, "could not disable SIGALRM timer: %m");
+ }
+}
+
/*****************************************************************************
* Signal handler
@@ -289,7 +313,6 @@ RegisterTimeout(TimeoutId id, timeout_handler handler)
static void
enable_timeout(TimeoutId id, TimestampTz now, TimestampTz fin_time)
{
- struct itimerval timeval;
int i;
/* Assert request is sane */
@@ -297,26 +320,6 @@ enable_timeout(TimeoutId id, TimestampTz now, TimestampTz fin_time)
Assert(all_timeouts[id].timeout_handler != NULL);
/*
- * Disable the timer if it is active; this avoids getting interrupted by
- * the signal handler and thereby possibly getting confused. We will
- * re-enable the interrupt below.
- *
- * If num_active_timeouts is zero, we don't have to call setitimer. There
- * should not be any pending interrupt, and even if there is, the worst
- * possible case is that the signal handler fires during schedule_alarm.
- * (If it fires at any point before insert_timeout has incremented
- * num_active_timeouts, it will do nothing.) In that case we could end up
- * scheduling a useless interrupt ... but when the interrupt does happen,
- * the signal handler will do nothing, so it's all good.
- */
- if (num_active_timeouts > 0)
- {
- MemSet(&timeval, 0, sizeof(struct itimerval));
- if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
- elog(FATAL, "could not disable SIGALRM timer: %m");
- }
-
- /*
* If this timeout was already active, momentarily disable it. We
* interpret the call as a directive to reschedule the timeout.
*/
@@ -345,11 +348,6 @@ enable_timeout(TimeoutId id, TimestampTz now, TimestampTz fin_time)
all_timeouts[id].start_time = now;
all_timeouts[id].fin_time = fin_time;
insert_timeout(id, i);
-
- /*
- * Set the timer.
- */
- schedule_alarm(now);
}
/*
@@ -366,7 +364,19 @@ enable_timeout_after(TimeoutId id, int delay_ms)
now = GetCurrentTimestamp();
fin_time = TimestampTzPlusMilliseconds(now, delay_ms);
+ /*
+ * Disable the timer if it is active; this avoids getting interrupted by
+ * the signal handler and thereby possibly getting confused. We will
+ * re-enable the interrupt below.
+ */
+ disable_alarm();
+
enable_timeout(id, now, fin_time);
+
+ /*
+ * Set the timer.
+ */
+ schedule_alarm(now);
}
/*
@@ -379,7 +389,72 @@ enable_timeout_after(TimeoutId id, int delay_ms)
void
enable_timeout_at(TimeoutId id, TimestampTz fin_time)
{
- enable_timeout(id, GetCurrentTimestamp(), fin_time);
+ TimestampTz now = GetCurrentTimestamp();
+
+ /*
+ * Disable the timer if it is active; this avoids getting interrupted by
+ * the signal handler and thereby possibly getting confused. We will
+ * re-enable the interrupt below.
+ */
+ disable_alarm();
+
+ enable_timeout(id, now, fin_time);
+
+ /*
+ * Set the timer.
+ */
+ schedule_alarm(now);
+}
+
+/*
+ * Enable multiple timeouts at once.
+ *
+ * It works like calling enable_timeout_after() or enable_timeout_at()
+ * separately. This is provided to minimize the number of setitimer() calls.
+ */
+void
+enable_timeouts(TimeoutParams *timeouts)
+{
+ TimestampTz now;
+ TimeoutId id;
+ int i;
+
+ now = GetCurrentTimestamp();
+
+ /*
+ * Disable the timer if it is active; this avoids getting interrupted by
+ * the signal handler and thereby possibly getting confused. We will
+ * re-enable the interrupt below.
+ */
+ disable_alarm();
+
+ for (i = 0, id = timeouts[i].id; id != NO_TIMEOUT; i++, id = timeouts[i].id)
+ {
+ TimestampTz fin_time;
+
+ switch (timeouts[i].type)
+ {
+ case TMPARAM_AFTER:
+ Assert(timeouts[i].delay_ms > 0);
+
+ fin_time = TimestampTzPlusMilliseconds(now, timeouts[i].delay_ms);
+ enable_timeout(id, now, fin_time);
+ break;
+
+ case TMPARAM_AT:
+ /* fin_time must be set */
+ Assert(timeouts[i].fin_time > 0);
+
+ enable_timeout(id, now, timeouts[i].fin_time);
+ break;
+
+ default:
+ elog(ERROR, "internal error: unknown timeout type");
+ break;
+ }
+ }
+
+ schedule_alarm(now);
}
/*
@@ -394,29 +469,13 @@ enable_timeout_at(TimeoutId id, TimestampTz fin_time)
void
disable_timeout(TimeoutId id, bool keep_indicator)
{
- struct itimerval timeval;
int i;
/* Assert request is sane */
Assert(all_timeouts_initialized);
Assert(all_timeouts[id].timeout_handler != NULL);
- /*
- * Disable the timer if it is active; this avoids getting interrupted by
- * the signal handler and thereby possibly getting confused. We will
- * re-enable the interrupt if necessary below.
- *
- * If num_active_timeouts is zero, we don't have to call setitimer. There
- * should not be any pending interrupt, and even if there is, the signal
- * handler will not do anything. In this situation the only thing we
- * really have to do is reset the timeout's indicator.
- */
- if (num_active_timeouts > 0)
- {
- MemSet(&timeval, 0, sizeof(struct itimerval));
- if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
- elog(FATAL, "could not disable SIGALRM timer: %m");
- }
+ disable_alarm();
/* Find the timeout and remove it from the active list. */
i = find_active_timeout(id);
@@ -427,9 +486,44 @@ disable_timeout(TimeoutId id, bool keep_indicator)
if (!keep_indicator)
all_timeouts[id].indicator = false;
- /* Now re-enable the timer, if necessary. */
- if (num_active_timeouts > 0)
- schedule_alarm(GetCurrentTimestamp());
+ schedule_alarm(GetCurrentTimestamp());
+}
+
+/*
+ * Cancel the specified timeouts at once.
+ *
+ * The timeouts' I've-been-fired indicator are reset,
+ * unless ->keep_indicator is true.
+ *
+ * After cancelling the specified timeouts, any other active timeout
+ * remains in force. It's not an error to disable a timeout that is
+ * not enabled.
+ */
+void
+disable_timeouts(TimeoutParams *timeouts)
+{
+ TimeoutId id;
+ int i, idx;
+
+ Assert(all_timeouts_initialized);
+ for (i = 0; timeouts[i].id != NO_TIMEOUT; i++)
+ {
+ Assert(all_timeouts[timeouts[i].id].timeout_handler != NULL);
+ }
+
+ disable_alarm();
+
+ for (i = 0, id = timeouts[i].id; id != NO_TIMEOUT; i++, id = timeouts[i].id)
+ {
+ idx = find_active_timeout(id);
+ if (idx >= 0)
+ remove_timeout_index(idx);
+
+ if (!timeouts[i].keep_indicator)
+ all_timeouts[id].indicator = false;
+ }
+
+ schedule_alarm(GetCurrentTimestamp());
}
/*
@@ -465,6 +559,17 @@ get_timeout_indicator(TimeoutId id)
}
/*
+ * Set the timeout's I've-been-fired indicator
+ * Can only be called when the actual timeout source
+ * is not active.
+ */
+void
+disable_timeout_indicator(TimeoutId id)
+{
+ all_timeouts[id].indicator = false;
+}
+
+/*
* Return the time when the timeout was most recently activated
*
* Note: will return 0 if timeout has never been activated in this process.
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index d571c35..b4a2140 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -222,6 +222,7 @@ extern PGPROC *PreparedXactProcs;
/* configurable options */
extern int DeadlockTimeout;
extern int StatementTimeout;
+extern int LockTimeout;
extern bool log_lock_waits;
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index e6f0afb..942329c 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -63,6 +63,7 @@ extern void assign_max_stack_depth(int newval, void *extra);
extern void die(SIGNAL_ARGS);
extern void quickdie(SIGNAL_ARGS) __attribute__((noreturn));
+extern void ProcessStatementCancel(void);
extern void StatementCancelHandler(SIGNAL_ARGS);
extern void FloatExceptionHandler(SIGNAL_ARGS) __attribute__((noreturn));
extern void RecoveryConflictInterrupt(ProcSignalReason reason); /* called from SIGUSR1
diff --git a/src/include/utils/timeout.h b/src/include/utils/timeout.h
index dd58c0e..a9c12c3 100644
--- a/src/include/utils/timeout.h
+++ b/src/include/utils/timeout.h
@@ -22,10 +22,14 @@
*/
typedef enum TimeoutId
{
+ /* End-of-records indicator for enable_timeouts() */
+ NO_TIMEOUT = -1,
+
/* Predefined timeout reasons */
- STARTUP_PACKET_TIMEOUT,
+ STARTUP_PACKET_TIMEOUT = 0,
DEADLOCK_TIMEOUT,
STATEMENT_TIMEOUT,
+ LOCK_TIMEOUT,
STANDBY_DEADLOCK_TIMEOUT,
STANDBY_TIMEOUT,
/* First user-definable timeout reason */
@@ -34,6 +38,22 @@ typedef enum TimeoutId
MAX_TIMEOUTS = 16
} TimeoutId;
+typedef enum TimeoutType {
+ TMPARAM_AFTER,
+ TMPARAM_AT
+} TimeoutType;
+
+/*
+ * Structure for setting multiple timeouts at once
+ */
+typedef struct {
+ TimeoutId id;
+ TimeoutType type;
+ int delay_ms;
+ TimestampTz fin_time;
+ bool keep_indicator;
+} TimeoutParams;
+
/* callback function signature */
typedef void (*timeout_handler) (void);
@@ -44,11 +64,14 @@ extern TimeoutId RegisterTimeout(TimeoutId id, timeout_handler handler);
/* timeout operation */
extern void enable_timeout_after(TimeoutId id, int delay_ms);
extern void enable_timeout_at(TimeoutId id, TimestampTz fin_time);
+extern void enable_timeouts(TimeoutParams *timeouts);
extern void disable_timeout(TimeoutId id, bool keep_indicator);
+extern void disable_timeouts(TimeoutParams *timeouts);
extern void disable_all_timeouts(bool keep_indicators);
/* accessors */
extern bool get_timeout_indicator(TimeoutId id);
+extern void disable_timeout_indicator(TimeoutId id);
extern TimestampTz get_timeout_start_time(TimeoutId id);
#endif /* TIMEOUT_H */
diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index c0b0864..009a6ca 100644
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -198,6 +198,11 @@ set statement_timeout to 2000;
SELECT * FROM pxtest3;
ERROR: canceling statement due to statement timeout
reset statement_timeout;
+-- pxtest3 should be locked because of the pending DROP
+set lock_timeout to 2000;
+SELECT * FROM pxtest3;
+ERROR: canceling statement due to lock timeout
+reset lock_timeout;
-- Disconnect, we will continue testing in a different backend
\c -
-- There should still be two prepared transactions
@@ -213,6 +218,11 @@ set statement_timeout to 2000;
SELECT * FROM pxtest3;
ERROR: canceling statement due to statement timeout
reset statement_timeout;
+-- pxtest3 should be locked because of the pending DROP
+set lock_timeout to 2000;
+SELECT * FROM pxtest3;
+ERROR: canceling statement due to lock timeout
+reset lock_timeout;
-- Commit table creation
COMMIT PREPARED 'regress-one';
\d pxtest2
diff --git a/src/test/regress/expected/prepared_xacts_1.out b/src/test/regress/expected/prepared_xacts_1.out
index 898f278..ec9ddcb 100644
--- a/src/test/regress/expected/prepared_xacts_1.out
+++ b/src/test/regress/expected/prepared_xacts_1.out
@@ -205,6 +205,14 @@ SELECT * FROM pxtest3;
(0 rows)
reset statement_timeout;
+-- pxtest3 should be locked because of the pending DROP
+set lock_timeout to 2000;
+SELECT * FROM pxtest3;
+ fff
+-----
+(0 rows)
+
+reset lock_timeout;
-- Disconnect, we will continue testing in a different backend
\c -
-- There should still be two prepared transactions
@@ -221,6 +229,14 @@ SELECT * FROM pxtest3;
(0 rows)
reset statement_timeout;
+-- pxtest3 should be locked because of the pending DROP
+set lock_timeout to 2000;
+SELECT * FROM pxtest3;
+ fff
+-----
+(0 rows)
+
+reset lock_timeout;
-- Commit table creation
COMMIT PREPARED 'regress-one';
ERROR: prepared transaction with identifier "regress-one" does not exist
diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql
index 7902152..872beed 100644
--- a/src/test/regress/sql/prepared_xacts.sql
+++ b/src/test/regress/sql/prepared_xacts.sql
@@ -126,6 +126,11 @@ set statement_timeout to 2000;
SELECT * FROM pxtest3;
reset statement_timeout;
+-- pxtest3 should be locked because of the pending DROP
+set lock_timeout to 2000;
+SELECT * FROM pxtest3;
+reset lock_timeout;
+
-- Disconnect, we will continue testing in a different backend
\c -
@@ -137,6 +142,11 @@ set statement_timeout to 2000;
SELECT * FROM pxtest3;
reset statement_timeout;
+-- pxtest3 should be locked because of the pending DROP
+set lock_timeout to 2000;
+SELECT * FROM pxtest3;
+reset lock_timeout;
+
-- Commit table creation
COMMIT PREPARED 'regress-one';
\d pxtest2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
