Add lock_timeout configuration parameter.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 17 Mar 2013 03:22:17 +0000 (23:22 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 17 Mar 2013 03:22:57 +0000 (23:22 -0400)
This GUC allows limiting the time spent waiting to acquire any one
heavyweight lock.

In support of this, improve the recently-added timeout infrastructure
to permit efficiently enabling or disabling multiple timeouts at once.
That reduces the performance hit from turning on lock_timeout, though
it's still not zero.

Zoltán Böszörményi, reviewed by Tom Lane,
Stephen Frost, and Hari Babu

16 files changed:
doc/src/sgml/config.sgml
src/backend/postmaster/autovacuum.c
src/backend/storage/ipc/standby.c
src/backend/storage/lmgr/proc.c
src/backend/tcop/postgres.c
src/backend/utils/init/postinit.c
src/backend/utils/misc/guc.c
src/backend/utils/misc/postgresql.conf.sample
src/backend/utils/misc/timeout.c
src/bin/pg_dump/pg_backup_archiver.c
src/bin/pg_dump/pg_dump.c
src/include/storage/proc.h
src/include/utils/timeout.h
src/test/isolation/expected/timeouts.out [new file with mode: 0644]
src/test/isolation/isolation_schedule
src/test/isolation/specs/timeouts.spec [new file with mode: 0644]

index 575b40b58df6cd040b2a7f192fe64a1e223d8421..8c520e1267ba158c18228dbb1899ac16aa20fa28 100644 (file)
@@ -5077,7 +5077,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </indexterm>
       <listitem>
        <para>
-        Abort any statement that takes over the specified number of
+        Abort any statement that takes more than the specified number of
         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
@@ -5086,8 +5086,42 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
 
        <para>
         Setting <varname>statement_timeout</> in
-        <filename>postgresql.conf</> is not recommended because it
-        affects all sessions.
+        <filename>postgresql.conf</> is not recommended because it would
+        affect all sessions.
+       </para>
+      </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 that waits longer than the specified number of
+        milliseconds while attempting to acquire a lock on a table, index,
+        row, or other database object.  The time limit applies separately to
+        each lock acquisition attempt.  The limit applies both to explicit
+        locking requests (such as <command>LOCK TABLE</>, or <command>SELECT
+        FOR UPDATE</> without <literal>NOWAIT</>) and to implicitly-acquired
+        locks.  If <varname>log_min_error_statement</> is set to
+        <literal>ERROR</> or lower, the statement that timed out will be
+        logged.  A value of zero (the default) turns this off.
+       </para>
+
+       <para>
+        Unlike <varname>statement_timeout</>, this timeout can only occur
+        while waiting for locks.  Note that if <varname>statement_timeout</>
+        is nonzero, it is rather pointless to set <varname>lock_timeout</> to
+        the same or larger value, since the statement timeout would always
+        trigger first.
+       </para>
+
+       <para>
+        Setting <varname>lock_timeout</> in
+        <filename>postgresql.conf</> is not recommended because it would
+        affect all sessions.
        </para>
       </listitem>
      </varlistentry>
index 00cb3f760d5add32a6e8ba926b183197c5dec14f..b4af6972c414e7388a79021eb495730384e501f2 100644 (file)
@@ -547,10 +547,11 @@ AutoVacLauncherMain(int argc, char *argv[])
    SetConfigOption("zero_damaged_pages", "false", PGC_SUSET, PGC_S_OVERRIDE);
 
    /*
-    * Force statement_timeout to zero to avoid a timeout setting from
-    * preventing regular maintenance from being executed.
+    * Force statement_timeout and lock_timeout to zero to avoid letting these
+    * settings prevent regular maintenance from being executed.
     */
    SetConfigOption("statement_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
+   SetConfigOption("lock_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
 
    /*
     * Force default_transaction_isolation to READ COMMITTED.  We don't want
@@ -1573,10 +1574,11 @@ AutoVacWorkerMain(int argc, char *argv[])
    SetConfigOption("zero_damaged_pages", "false", PGC_SUSET, PGC_S_OVERRIDE);
 
    /*
-    * Force statement_timeout to zero to avoid a timeout setting from
-    * preventing regular maintenance from being executed.
+    * Force statement_timeout and lock_timeout to zero to avoid letting these
+    * settings prevent regular maintenance from being executed.
     */
    SetConfigOption("statement_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
+   SetConfigOption("lock_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE);
 
    /*
     * Force default_transaction_isolation to READ COMMITTED.  We don't want
index a903f127660804969ffcae15cf3d9b7200c961ea..fcf08f42b36789f5f61c6e766adb8fc00f862317 100644 (file)
@@ -428,8 +428,15 @@ ResolveRecoveryConflictWithBufferPin(void)
         * 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);
+       EnableTimeoutParams timeouts[2];
+
+       timeouts[0].id = STANDBY_TIMEOUT;
+       timeouts[0].type = TMPARAM_AT;
+       timeouts[0].fin_time = ltime;
+       timeouts[1].id = STANDBY_DEADLOCK_TIMEOUT;
+       timeouts[1].type = TMPARAM_AFTER;
+       timeouts[1].delay_ms = DeadlockTimeout;
+       enable_timeouts(timeouts, 2);
    }
 
    /* Wait to be signaled by UnpinBuffer() */
index 2e012fad11fe3897d3750a08af27f316f0bca008..5809a797986734e4b9a6a9d70dc58ba7a67145a7 100644 (file)
@@ -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,7 @@ void
 LockErrorCleanup(void)
 {
    LWLockId    partitionLock;
+   DisableTimeoutParams timeouts[2];
 
    AbortStrongLockAcquire();
 
@@ -672,8 +674,19 @@ 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).  Note we must preserve the LOCK_TIMEOUT
+    * indicator flag, since this function is executed before
+    * ProcessInterrupts when responding to SIGINT; else we'd lose the
+    * knowledge that the SIGINT came from a lock timeout and not an external
+    * source.
+    */
+   timeouts[0].id = DEADLOCK_TIMEOUT;
+   timeouts[0].keep_indicator = false;
+   timeouts[1].id = LOCK_TIMEOUT;
+   timeouts[1].keep_indicator = true;
+   disable_timeouts(timeouts, 2);
 
    /* Unlink myself from the wait queue, if on it (might not be anymore!) */
    partitionLock = LockHashPartitionLock(lockAwaited->hashcode);
@@ -1072,8 +1085,24 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
     *
     * 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 LockTimeout is set, also enable the timeout for that.  We can save a
+    * few cycles by enabling both timeout sources in one call.
     */
-   enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
+   if (LockTimeout > 0)
+   {
+       EnableTimeoutParams timeouts[2];
+
+       timeouts[0].id = DEADLOCK_TIMEOUT;
+       timeouts[0].type = TMPARAM_AFTER;
+       timeouts[0].delay_ms = DeadlockTimeout;
+       timeouts[1].id = LOCK_TIMEOUT;
+       timeouts[1].type = TMPARAM_AFTER;
+       timeouts[1].delay_ms = LockTimeout;
+       enable_timeouts(timeouts, 2);
+   }
+   else
+       enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
 
    /*
     * If someone wakes us between LWLockRelease and PGSemaphoreLock,
@@ -1240,9 +1269,20 @@ 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);
+   if (LockTimeout > 0)
+   {
+       DisableTimeoutParams timeouts[2];
+
+       timeouts[0].id = DEADLOCK_TIMEOUT;
+       timeouts[0].keep_indicator = false;
+       timeouts[1].id = LOCK_TIMEOUT;
+       timeouts[1].keep_indicator = false;
+       disable_timeouts(timeouts, 2);
+   }
+   else
+       disable_timeout(DEADLOCK_TIMEOUT, false);
 
    /*
     * Re-acquire the lock table's partition lock.  We have to do this to hold
index 407c548cf8f07b7f217f88134de57ebf8a1adb00..587d065f1ccb7bf1ca1ba3d05b133dacf0099ab2 100644 (file)
@@ -2883,7 +2883,22 @@ ProcessInterrupts(void)
                    (errcode(ERRCODE_QUERY_CANCELED),
                     errmsg("canceling authentication due to timeout")));
        }
-       if (get_timeout_indicator(STATEMENT_TIMEOUT))
+
+       /*
+        * If LOCK_TIMEOUT and STATEMENT_TIMEOUT indicators are both set, we
+        * prefer to report the former; but be sure to clear both.
+        */
+       if (get_timeout_indicator(LOCK_TIMEOUT, true))
+       {
+           ImmediateInterruptOK = false;       /* not idle anymore */
+           (void) get_timeout_indicator(STATEMENT_TIMEOUT, true);
+           DisableNotifyInterrupt();
+           DisableCatchupInterrupt();
+           ereport(ERROR,
+                   (errcode(ERRCODE_QUERY_CANCELED),
+                    errmsg("canceling statement due to lock timeout")));
+       }
+       if (get_timeout_indicator(STATEMENT_TIMEOUT, true))
        {
            ImmediateInterruptOK = false;       /* not idle anymore */
            DisableNotifyInterrupt();
index 84270061d8a9c96963da03d83642ddd89f1bc862..5f8f98b5e03ebeb47f337d5fadfb6efdd883abf3 100644 (file)
@@ -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);
    }
 
    /*
@@ -1052,6 +1054,22 @@ StatementTimeoutHandler(void)
    kill(MyProcPid, SIGINT);
 }
 
+/*
+ * LOCK_TIMEOUT handler: trigger a query-cancel interrupt.
+ *
+ * This is identical to StatementTimeoutHandler, but since it's so short,
+ * we might as well keep the two functions separate for clarity.
+ */
+static void
+LockTimeoutHandler(void)
+{
+#ifdef HAVE_SETSID
+   /* try to signal whole process group */
+   kill(-MyProcPid, SIGINT);
+#endif
+   kill(MyProcPid, SIGINT);
+}
+
 
 /*
  * Returns true if at least one role is defined in this database cluster.
index 98149fc10f6a3cbd8c973aba9c27de3cee11ebe4..5246fc5b2015aa92b3dd32bff2ade6b58acd1789 100644 (file)
@@ -1861,6 +1861,17 @@ static struct config_int ConfigureNamesInt[] =
        NULL, NULL, NULL
    },
 
+   {
+       {"lock_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT,
+           gettext_noop("Sets the maximum allowed duration of any wait for a lock."),
+           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."),
index 62aea2f583e0b7ac3c9b7ae6b263f3adeab6e643..307b456f0350b0dde25754dd0a2bfe9cdc8d79bc 100644 (file)
 #default_transaction_deferrable = off
 #session_replication_role = 'origin'
 #statement_timeout = 0         # in milliseconds, 0 is disabled
+#lock_timeout = 0          # in milliseconds, 0 is disabled
 #vacuum_freeze_min_age = 50000000
 #vacuum_freeze_table_age = 150000000
 #bytea_output = 'hex'          # hex, escape
index 944c9a7bcb134a99b99d5a4a1f5fa99b3806b45b..2ee6e00ed28c23b11bd7b7713daf682745db163f 100644 (file)
@@ -31,7 +31,7 @@ typedef struct timeout_params
    volatile bool indicator;    /* true if timeout has occurred */
 
    /* callback function for timeout, or NULL if timeout not registered */
-   timeout_handler timeout_handler;
+   timeout_handler_proc timeout_handler;
 
    TimestampTz start_time;     /* time that timeout was last activated */
    TimestampTz fin_time;       /* if active, time it is due to fire */
@@ -55,9 +55,48 @@ static timeout_params *volatile active_timeouts[MAX_TIMEOUTS];
  * Internal helper functions
  *
  * For all of these, it is caller's responsibility to protect them from
- * interruption by the signal handler.
+ * interruption by the signal handler. Generally, call disable_alarm()
+ * first to prevent interruption, then update state, and last call
+ * schedule_alarm(), which will re-enable the interrupt if needed.
  *****************************************************************************/
 
+/*
+ * Disable alarm interrupts
+ *
+ * multi_insert must be true if the caller intends to activate multiple new
+ * timeouts.  Otherwise it should be false.
+ */
+static void
+disable_alarm(bool multi_insert)
+{
+   struct itimerval timeval;
+
+   /*
+    * If num_active_timeouts is zero, and multi_insert is false, 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,
+    * since it sees no active timeouts.)  In that case we could end up
+    * scheduling a useless interrupt ... but when the extra interrupt does
+    * happen, the signal handler will do nothing, so it's all good.
+    *
+    * However, if the caller intends to do anything more after first calling
+    * insert_timeout, the above argument breaks down, since the signal
+    * handler could interrupt the subsequent operations leading to corrupt
+    * state.  Out of an abundance of caution, we forcibly disable the timer
+    * even though it should be off already, just to be sure.  Even though
+    * this setitimer call is probably useless, we're still ahead of the game
+    * compared to scheduling two or more timeouts independently.
+    */
+   if (multi_insert || 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");
+   }
+}
+
 /*
  * Find the index of a given timeout reason in the active array.
  * If it's not there, return -1.
@@ -94,7 +133,7 @@ insert_timeout(TimeoutId id, int index)
 
    active_timeouts[index] = &all_timeouts[id];
 
-   /* NB: this must be the last step, see comments in enable_timeout */
+   /* NB: this must be the last step, see comments in disable_alarm */
    num_active_timeouts++;
 }
 
@@ -116,6 +155,49 @@ remove_timeout_index(int index)
    num_active_timeouts--;
 }
 
+/*
+ * Enable the specified timeout reason
+ */
+static void
+enable_timeout(TimeoutId id, TimestampTz now, TimestampTz fin_time)
+{
+   int         i;
+
+   /* Assert request is sane */
+   Assert(all_timeouts_initialized);
+   Assert(all_timeouts[id].timeout_handler != NULL);
+
+   /*
+    * If this timeout was already active, momentarily disable it.  We
+    * interpret the call as a directive to reschedule the timeout.
+    */
+   i = find_active_timeout(id);
+   if (i >= 0)
+       remove_timeout_index(i);
+
+   /*
+    * Find out the index where to insert the new timeout.  We sort by
+    * fin_time, and for equal fin_time by priority.
+    */
+   for (i = 0; i < num_active_timeouts; i++)
+   {
+       timeout_params *old_timeout = active_timeouts[i];
+
+       if (fin_time < old_timeout->fin_time)
+           break;
+       if (fin_time == old_timeout->fin_time && id < old_timeout->index)
+           break;
+   }
+
+   /*
+    * Mark the timeout active, and insert it into the active list.
+    */
+   all_timeouts[id].indicator = false;
+   all_timeouts[id].start_time = now;
+   all_timeouts[id].fin_time = fin_time;
+   insert_timeout(id, i);
+}
+
 /*
  * Schedule alarm for the next active timeout, if any
  *
@@ -260,7 +342,7 @@ InitializeTimeouts(void)
  * return a timeout ID.
  */
 TimeoutId
-RegisterTimeout(TimeoutId id, timeout_handler handler)
+RegisterTimeout(TimeoutId id, timeout_handler_proc handler)
 {
    Assert(all_timeouts_initialized);
 
@@ -283,75 +365,6 @@ RegisterTimeout(TimeoutId id, timeout_handler handler)
    return id;
 }
 
-/*
- * Enable the specified timeout reason
- */
-static void
-enable_timeout(TimeoutId id, TimestampTz now, TimestampTz fin_time)
-{
-   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 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.
-    */
-   i = find_active_timeout(id);
-   if (i >= 0)
-       remove_timeout_index(i);
-
-   /*
-    * Find out the index where to insert the new timeout.  We sort by
-    * fin_time, and for equal fin_time by priority.
-    */
-   for (i = 0; i < num_active_timeouts; i++)
-   {
-       timeout_params *old_timeout = active_timeouts[i];
-
-       if (fin_time < old_timeout->fin_time)
-           break;
-       if (fin_time == old_timeout->fin_time && id < old_timeout->index)
-           break;
-   }
-
-   /*
-    * Activate the timeout.
-    */
-   all_timeouts[id].indicator = false;
-   all_timeouts[id].start_time = now;
-   all_timeouts[id].fin_time = fin_time;
-   insert_timeout(id, i);
-
-   /*
-    * Set the timer.
-    */
-   schedule_alarm(now);
-}
-
 /*
  * Enable the specified timeout to fire after the specified delay.
  *
@@ -363,10 +376,16 @@ enable_timeout_after(TimeoutId id, int delay_ms)
    TimestampTz now;
    TimestampTz fin_time;
 
+   /* Disable timeout interrupts for safety. */
+   disable_alarm(false);
+
+   /* Queue the timeout at the appropriate time. */
    now = GetCurrentTimestamp();
    fin_time = TimestampTzPlusMilliseconds(now, delay_ms);
-
    enable_timeout(id, now, fin_time);
+
+   /* Set the timer interrupt. */
+   schedule_alarm(now);
 }
 
 /*
@@ -379,7 +398,64 @@ 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;
+
+   /* Disable timeout interrupts for safety. */
+   disable_alarm(false);
+
+   /* Queue the timeout at the appropriate time. */
+   now = GetCurrentTimestamp();
+   enable_timeout(id, now, fin_time);
+
+   /* Set the timer interrupt. */
+   schedule_alarm(now);
+}
+
+/*
+ * Enable multiple timeouts at once.
+ *
+ * This works like calling enable_timeout_after() and/or enable_timeout_at()
+ * multiple times. Use this to reduce the number of GetCurrentTimestamp()
+ * and setitimer() calls needed to establish multiple timeouts.
+ */
+void
+enable_timeouts(const EnableTimeoutParams *timeouts, int count)
+{
+   TimestampTz now;
+   int         i;
+
+   /* Disable timeout interrupts for safety. */
+   disable_alarm(count > 1);
+
+   /* Queue the timeout(s) at the appropriate times. */
+   now = GetCurrentTimestamp();
+
+   for (i = 0; i < count; i++)
+   {
+       TimeoutId   id = timeouts[i].id;
+       TimestampTz fin_time;
+
+       switch (timeouts[i].type)
+       {
+           case TMPARAM_AFTER:
+               fin_time = TimestampTzPlusMilliseconds(now,
+                                                      timeouts[i].delay_ms);
+               enable_timeout(id, now, fin_time);
+               break;
+
+           case TMPARAM_AT:
+               enable_timeout(id, now, timeouts[i].fin_time);
+               break;
+
+           default:
+               elog(ERROR, "unrecognized timeout type %d",
+                    (int) timeouts[i].type);
+               break;
+       }
+   }
+
+   /* Set the timer interrupt. */
+   schedule_alarm(now);
 }
 
 /*
@@ -394,29 +470,14 @@ 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 timeout interrupts for safety. */
+   disable_alarm(false);
 
    /* Find the timeout and remove it from the active list. */
    i = find_active_timeout(id);
@@ -427,7 +488,48 @@ disable_timeout(TimeoutId id, bool keep_indicator)
    if (!keep_indicator)
        all_timeouts[id].indicator = false;
 
-   /* Now re-enable the timer, if necessary. */
+   /* Reschedule the interrupt, if any timeouts remain active. */
+   if (num_active_timeouts > 0)
+       schedule_alarm(GetCurrentTimestamp());
+}
+
+/*
+ * Cancel multiple timeouts at once.
+ *
+ * The timeouts' I've-been-fired indicators are reset,
+ * unless timeouts[i].keep_indicator is true.
+ *
+ * This works like calling disable_timeout() multiple times.
+ * Use this to reduce the number of GetCurrentTimestamp()
+ * and setitimer() calls needed to cancel multiple timeouts.
+ */
+void
+disable_timeouts(const DisableTimeoutParams *timeouts, int count)
+{
+   int         i;
+
+   Assert(all_timeouts_initialized);
+
+   /* Disable timeout interrupts for safety. */
+   disable_alarm(false);
+
+   /* Cancel the timeout(s). */
+   for (i = 0; i < count; i++)
+   {
+       TimeoutId   id = timeouts[i].id;
+       int         idx;
+
+       Assert(all_timeouts[id].timeout_handler != NULL);
+
+       idx = find_active_timeout(id);
+       if (idx >= 0)
+           remove_timeout_index(idx);
+
+       if (!timeouts[i].keep_indicator)
+           all_timeouts[id].indicator = false;
+   }
+
+   /* Reschedule the interrupt, if any timeouts remain active. */
    if (num_active_timeouts > 0)
        schedule_alarm(GetCurrentTimestamp());
 }
@@ -442,6 +544,7 @@ disable_all_timeouts(bool keep_indicators)
    struct itimerval timeval;
    int         i;
 
+   /* Forcibly reset the timer, whether we think it's active or not */
    MemSet(&timeval, 0, sizeof(struct itimerval));
    if (setitimer(ITIMER_REAL, &timeval, NULL) != 0)
        elog(FATAL, "could not disable SIGALRM timer: %m");
@@ -457,11 +560,21 @@ disable_all_timeouts(bool keep_indicators)
 
 /*
  * Return the timeout's I've-been-fired indicator
+ *
+ * If reset_indicator is true, reset the indicator when returning true.
+ * To avoid missing timeouts due to race conditions, we are careful not to
+ * reset the indicator when returning false.
  */
 bool
-get_timeout_indicator(TimeoutId id)
+get_timeout_indicator(TimeoutId id, bool reset_indicator)
 {
-   return all_timeouts[id].indicator;
+   if (all_timeouts[id].indicator)
+   {
+       if (reset_indicator)
+           all_timeouts[id].indicator = false;
+       return true;
+   }
+   return false;
 }
 
 /*
index d500bfd234b6515e2318861a01bdb427dd470158..19d12788d9d0d202adbc116a30c81da6fcfa609e 100644 (file)
@@ -2592,9 +2592,12 @@ _tocEntryIsACL(TocEntry *te)
 static void
 _doSetFixedOutputState(ArchiveHandle *AH)
 {
-   /* Disable statement_timeout in archive for pg_restore/psql  */
+   /* Disable statement_timeout since restore is probably slow */
    ahprintf(AH, "SET statement_timeout = 0;\n");
 
+   /* Likewise for lock_timeout */
+   ahprintf(AH, "SET lock_timeout = 0;\n");
+
    /* Select the correct character set encoding */
    ahprintf(AH, "SET client_encoding = '%s';\n",
             pg_encoding_to_char(AH->public.encoding));
index 94584292dc49ecda772b63b3bd4d65f1995630b8..093be9e16d0671c232620817cde7bd972cf1921c 100644 (file)
@@ -957,6 +957,8 @@ setup_connection(Archive *AH, const char *dumpencoding, char *use_role)
     */
    if (AH->remoteVersion >= 70300)
        ExecuteSqlStatement(AH, "SET statement_timeout = 0");
+   if (AH->remoteVersion >= 90300)
+       ExecuteSqlStatement(AH, "SET lock_timeout = 0");
 
    /*
     * Quote all identifiers, if requested.
index d571c35ab6b947e1a1a9aa8f9637297c5b2c9096..3b04d3c1fb1c25701c6fd8d8dee5d1a4a4199eb5 100644 (file)
@@ -168,8 +168,8 @@ typedef struct PGXACT
 
    uint8       vacuumFlags;    /* vacuum-related flags, see above */
    bool        overflowed;
-   bool        delayChkpt;     /* true if this proc delays checkpoint start */
-                               /* previously called InCommit */
+   bool        delayChkpt;     /* true if this proc delays checkpoint start;
+                                * previously called InCommit */
 
    uint8       nxids;
 } PGXACT;
@@ -222,6 +222,7 @@ extern PGPROC *PreparedXactProcs;
 /* configurable options */
 extern int DeadlockTimeout;
 extern int StatementTimeout;
+extern int LockTimeout;
 extern bool log_lock_waits;
 
 
index dd58c0e1034e4e543c5bbbd0df2f8e82a431632d..06bcc5a0f3d92aabdb86b51570a398b19839bf55 100644 (file)
@@ -25,6 +25,7 @@ typedef enum TimeoutId
    /* Predefined timeout reasons */
    STARTUP_PACKET_TIMEOUT,
    DEADLOCK_TIMEOUT,
+   LOCK_TIMEOUT,
    STATEMENT_TIMEOUT,
    STANDBY_DEADLOCK_TIMEOUT,
    STANDBY_TIMEOUT,
@@ -35,20 +36,48 @@ typedef enum TimeoutId
 } TimeoutId;
 
 /* callback function signature */
-typedef void (*timeout_handler) (void);
+typedef void (*timeout_handler_proc) (void);
+
+/*
+ * Parameter structure for setting multiple timeouts at once
+ */
+typedef enum TimeoutType
+{
+   TMPARAM_AFTER,
+   TMPARAM_AT
+} TimeoutType;
+
+typedef struct
+{
+   TimeoutId   id;             /* timeout to set */
+   TimeoutType type;           /* TMPARAM_AFTER or TMPARAM_AT */
+   int         delay_ms;       /* only used for TMPARAM_AFTER */
+   TimestampTz fin_time;       /* only used for TMPARAM_AT */
+} EnableTimeoutParams;
+
+/*
+ * Parameter structure for clearing multiple timeouts at once
+ */
+typedef struct
+{
+   TimeoutId   id;             /* timeout to clear */
+   bool        keep_indicator; /* keep the indicator flag? */
+} DisableTimeoutParams;
 
 /* timeout setup */
 extern void InitializeTimeouts(void);
-extern TimeoutId RegisterTimeout(TimeoutId id, timeout_handler handler);
+extern TimeoutId RegisterTimeout(TimeoutId id, timeout_handler_proc 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(const EnableTimeoutParams *timeouts, int count);
 extern void disable_timeout(TimeoutId id, bool keep_indicator);
+extern void disable_timeouts(const DisableTimeoutParams *timeouts, int count);
 extern void disable_all_timeouts(bool keep_indicators);
 
 /* accessors */
-extern bool get_timeout_indicator(TimeoutId id);
+extern bool get_timeout_indicator(TimeoutId id, bool reset_indicator);
 extern TimestampTz get_timeout_start_time(TimeoutId id);
 
 #endif   /* TIMEOUT_H */
diff --git a/src/test/isolation/expected/timeouts.out b/src/test/isolation/expected/timeouts.out
new file mode 100644 (file)
index 0000000..0ad792a
--- /dev/null
@@ -0,0 +1,73 @@
+Parsed test spec with 2 sessions
+
+starting permutation: rdtbl sto locktbl
+step rdtbl: SELECT * FROM accounts;
+accountid      balance        
+
+checking       600            
+savings        600            
+step sto: SET statement_timeout = 1000;
+step locktbl: LOCK TABLE accounts; <waiting ...>
+step locktbl: <... completed>
+ERROR:  canceling statement due to statement timeout
+
+starting permutation: rdtbl lto locktbl
+step rdtbl: SELECT * FROM accounts;
+accountid      balance        
+
+checking       600            
+savings        600            
+step lto: SET lock_timeout = 1000;
+step locktbl: LOCK TABLE accounts; <waiting ...>
+step locktbl: <... completed>
+ERROR:  canceling statement due to lock timeout
+
+starting permutation: rdtbl lsto locktbl
+step rdtbl: SELECT * FROM accounts;
+accountid      balance        
+
+checking       600            
+savings        600            
+step lsto: SET lock_timeout = 1000; SET statement_timeout = 2000;
+step locktbl: LOCK TABLE accounts; <waiting ...>
+step locktbl: <... completed>
+ERROR:  canceling statement due to lock timeout
+
+starting permutation: rdtbl slto locktbl
+step rdtbl: SELECT * FROM accounts;
+accountid      balance        
+
+checking       600            
+savings        600            
+step slto: SET lock_timeout = 2000; SET statement_timeout = 1000;
+step locktbl: LOCK TABLE accounts; <waiting ...>
+step locktbl: <... completed>
+ERROR:  canceling statement due to statement timeout
+
+starting permutation: wrtbl sto update
+step wrtbl: UPDATE accounts SET balance = balance + 100;
+step sto: SET statement_timeout = 1000;
+step update: DELETE FROM accounts WHERE accountid = 'checking'; <waiting ...>
+step update: <... completed>
+ERROR:  canceling statement due to statement timeout
+
+starting permutation: wrtbl lto update
+step wrtbl: UPDATE accounts SET balance = balance + 100;
+step lto: SET lock_timeout = 1000;
+step update: DELETE FROM accounts WHERE accountid = 'checking'; <waiting ...>
+step update: <... completed>
+ERROR:  canceling statement due to lock timeout
+
+starting permutation: wrtbl lsto update
+step wrtbl: UPDATE accounts SET balance = balance + 100;
+step lsto: SET lock_timeout = 1000; SET statement_timeout = 2000;
+step update: DELETE FROM accounts WHERE accountid = 'checking'; <waiting ...>
+step update: <... completed>
+ERROR:  canceling statement due to lock timeout
+
+starting permutation: wrtbl slto update
+step wrtbl: UPDATE accounts SET balance = balance + 100;
+step slto: SET lock_timeout = 2000; SET statement_timeout = 1000;
+step update: DELETE FROM accounts WHERE accountid = 'checking'; <waiting ...>
+step update: <... completed>
+ERROR:  canceling statement due to statement timeout
index c4d6719de6da19d8d525382389f5127c62323b12..081e11f2a1e43dd65c1f7082e4e14135374f1a42 100644 (file)
@@ -20,3 +20,4 @@ test: delete-abort-savept
 test: delete-abort-savept-2
 test: aborted-keyrevoke
 test: drop-index-concurrently-1
+test: timeouts
diff --git a/src/test/isolation/specs/timeouts.spec b/src/test/isolation/specs/timeouts.spec
new file mode 100644 (file)
index 0000000..000b50c
--- /dev/null
@@ -0,0 +1,45 @@
+# Simple tests for statement_timeout and lock_timeout features
+
+setup
+{
+ CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null);
+ INSERT INTO accounts VALUES ('checking', 600), ('savings', 600);
+}
+
+teardown
+{
+ DROP TABLE accounts;
+}
+
+session "s1"
+setup      { BEGIN ISOLATION LEVEL READ COMMITTED; }
+step "rdtbl"   { SELECT * FROM accounts; }
+step "wrtbl"   { UPDATE accounts SET balance = balance + 100; }
+teardown   { ABORT; }
+
+session "s2"
+setup      { BEGIN ISOLATION LEVEL READ COMMITTED; }
+step "sto" { SET statement_timeout = 1000; }
+step "lto" { SET lock_timeout = 1000; }
+step "lsto"    { SET lock_timeout = 1000; SET statement_timeout = 2000; }
+step "slto"    { SET lock_timeout = 2000; SET statement_timeout = 1000; }
+step "locktbl" { LOCK TABLE accounts; }
+step "update"  { DELETE FROM accounts WHERE accountid = 'checking'; }
+teardown   { ABORT; }
+
+# statement timeout, table-level lock
+permutation "rdtbl" "sto" "locktbl"
+# lock timeout, table-level lock
+permutation "rdtbl" "lto" "locktbl"
+# lock timeout expires first, table-level lock
+permutation "rdtbl" "lsto" "locktbl"
+# statement timeout expires first, table-level lock
+permutation "rdtbl" "slto" "locktbl"
+# statement timeout, row-level lock
+permutation "wrtbl" "sto" "update"
+# lock timeout, row-level lock
+permutation "wrtbl" "lto" "update"
+# lock timeout expires first, row-level lock
+permutation "wrtbl" "lsto" "update"
+# statement timeout expires first, row-level lock
+permutation "wrtbl" "slto" "update"