Hi,

new version with a lot more cleanup is attached.

2012-07-22 22:03 keltezéssel, Boszormenyi Zoltan írta:
Attached is the revised (and a lot leaner, more generic) lock timeout patch,
which introduces new functionality for the timeout registration framework.
The new functionality is called "extra timeouts", better naming is welcome.
Instead of only the previously defined (deadlock and statement) timeouts,
the "extra" timeouts can also be activated from within ProcSleep() in a linked
way.

This "mini-framework" is now called "lock manager timeouts" and
both deadlock timeout and the new lock timeout belong to it.
The little piece of standalone code managing these are in
storage/lmgr/lmgrtimeout.c.

There is no PGSemaphoreTimedLock() any more. Instead,
PGSemaphoreLock() gained a new function argument for
checking timeouts. This has three advantages:
- There is only one PGSemaphoreLock() implementation and bug fixes
  like ada8fa08fc6cf5f199b6df935b4d0a730aaa4fec don't need to
  touch several places.
- There is no layering violation between pg_sema.c and proc.c.
- The extra function can check other type of conditions from different
  callers, should the need arise.

Best regards,
Zoltán Böszörményi

--
----------------------------------
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 -durpN postgresql/src/backend/port/ipc_test.c postgresql.1/src/backend/port/ipc_test.c
--- postgresql/src/backend/port/ipc_test.c	2012-04-16 19:57:22.437915477 +0200
+++ postgresql.1/src/backend/port/ipc_test.c	2012-08-21 15:53:50.059329927 +0200
@@ -240,7 +240,7 @@ main(int argc, char **argv)
 	printf("Testing Lock ... ");
 	fflush(stdout);
 
-	PGSemaphoreLock(&storage->sem, false);
+	PGSemaphoreLock(&storage->sem, false, NULL);
 
 	printf("OK\n");
 
@@ -262,8 +262,8 @@ main(int argc, char **argv)
 	PGSemaphoreUnlock(&storage->sem);
 	PGSemaphoreUnlock(&storage->sem);
 
-	PGSemaphoreLock(&storage->sem, false);
-	PGSemaphoreLock(&storage->sem, false);
+	PGSemaphoreLock(&storage->sem, false, NULL);
+	PGSemaphoreLock(&storage->sem, false, NULL);
 
 	printf("OK\n");
 
@@ -311,7 +311,7 @@ main(int argc, char **argv)
 	printf("Waiting for child (should wait 3 sec here) ... ");
 	fflush(stdout);
 
-	PGSemaphoreLock(&storage->sem, false);
+	PGSemaphoreLock(&storage->sem, false, NULL);
 
 	printf("OK\n");
 
diff -durpN postgresql/src/backend/port/posix_sema.c postgresql.1/src/backend/port/posix_sema.c
--- postgresql/src/backend/port/posix_sema.c	2012-04-16 19:57:22.438915489 +0200
+++ postgresql.1/src/backend/port/posix_sema.c	2012-08-21 15:49:26.215579665 +0200
@@ -236,9 +236,11 @@ PGSemaphoreReset(PGSemaphore sema)
  * Lock a semaphore (decrement count), blocking if count would be < 0
  */
 void
-PGSemaphoreLock(PGSemaphore sema, bool interruptOK)
+PGSemaphoreLock(PGSemaphore sema, bool interruptOK,
+					PGSemaphoreCondition condition_checker)
 {
 	int			errStatus;
+	bool			condition = false;
 
 	/*
 	 * See notes in sysv_sema.c's implementation of PGSemaphoreLock. Just as
@@ -252,8 +254,12 @@ PGSemaphoreLock(PGSemaphore sema, bool i
 		CHECK_FOR_INTERRUPTS();
 		errStatus = sem_wait(PG_SEM_REF(sema));
 		ImmediateInterruptOK = false;
-	} while (errStatus < 0 && errno == EINTR);
+		if (condition_checker)
+			condition = condition_checker();
+	} while (errStatus < 0 && errno == EINTR && !condition);
 
+	if (condition)
+		return;
 	if (errStatus < 0)
 		elog(FATAL, "sem_wait failed: %m");
 }
diff -durpN postgresql/src/backend/port/sysv_sema.c postgresql.1/src/backend/port/sysv_sema.c
--- postgresql/src/backend/port/sysv_sema.c	2012-05-14 08:20:56.284830580 +0200
+++ postgresql.1/src/backend/port/sysv_sema.c	2012-08-21 15:49:26.991584804 +0200
@@ -358,9 +358,11 @@ PGSemaphoreReset(PGSemaphore sema)
  * Lock a semaphore (decrement count), blocking if count would be < 0
  */
 void
-PGSemaphoreLock(PGSemaphore sema, bool interruptOK)
+PGSemaphoreLock(PGSemaphore sema, bool interruptOK,
+					PGSemaphoreCondition condition_checker)
 {
 	int			errStatus;
+	bool			condition = false;
 	struct sembuf sops;
 
 	sops.sem_op = -1;			/* decrement */
@@ -414,8 +416,12 @@ PGSemaphoreLock(PGSemaphore sema, bool i
 		CHECK_FOR_INTERRUPTS();
 		errStatus = semop(sema->semId, &sops, 1);
 		ImmediateInterruptOK = false;
-	} while (errStatus < 0 && errno == EINTR);
+		if (condition_checker)
+			condition = condition_checker();
+	} while (errStatus < 0 && errno == EINTR && !condition);
 
+	if (condition)
+		return;
 	if (errStatus < 0)
 		elog(FATAL, "semop(id=%d) failed: %m", sema->semId);
 }
diff -durpN postgresql/src/backend/port/win32_sema.c postgresql.1/src/backend/port/win32_sema.c
--- postgresql/src/backend/port/win32_sema.c	2012-06-11 06:22:48.137921483 +0200
+++ postgresql.1/src/backend/port/win32_sema.c	2012-08-21 15:49:24.921571070 +0200
@@ -116,10 +116,12 @@ PGSemaphoreReset(PGSemaphore sema)
  * Serve the interrupt if interruptOK is true.
  */
 void
-PGSemaphoreLock(PGSemaphore sema, bool interruptOK)
+PGSemaphoreLock(PGSemaphore sema, bool interruptOK,
+					PGSemaphoreCondition condition_checker)
 {
 	DWORD		ret;
 	HANDLE		wh[2];
+	bool		condition = false;
 
 	/*
 	 * Note: pgwin32_signal_event should be first to ensure that it will be
@@ -158,8 +160,12 @@ PGSemaphoreLock(PGSemaphore sema, bool i
 			errno = EIDRM;
 
 		ImmediateInterruptOK = false;
-	} while (errno == EINTR);
+		if (condition_checker)
+			condition = condition_checker();
+	} while (errno == EINTR && !condition);
 
+	if (condition)
+		return;
 	if (errno != 0)
 		ereport(FATAL,
 		(errmsg("could not lock semaphore: error code %lu", GetLastError())));
diff -durpN postgresql/src/backend/storage/lmgr/deadlock.c postgresql.1/src/backend/storage/lmgr/deadlock.c
--- postgresql/src/backend/storage/lmgr/deadlock.c	2012-07-27 10:19:36.932331508 +0200
+++ postgresql.1/src/backend/storage/lmgr/deadlock.c	2012-08-21 15:08:16.728428766 +0200
@@ -31,6 +31,7 @@
 #include "storage/lmgr.h"
 #include "storage/proc.h"
 #include "utils/memutils.h"
+#include "utils/timeout.h"
 
 
 /* One edge in the waits-for graph */
@@ -897,6 +898,9 @@ DeadLockReport(void)
 	StringInfoData locktagbuf;
 	int			i;
 
+	if (!DeadLockTimeoutCondition())
+		return;
+
 	initStringInfo(&clientbuf);
 	initStringInfo(&logbuf);
 	initStringInfo(&locktagbuf);
diff -durpN postgresql/src/backend/storage/lmgr/lmgrtimeout.c postgresql.1/src/backend/storage/lmgr/lmgrtimeout.c
--- postgresql/src/backend/storage/lmgr/lmgrtimeout.c	1970-01-01 01:00:00.000000000 +0100
+++ postgresql.1/src/backend/storage/lmgr/lmgrtimeout.c	2012-08-21 13:05:19.214992078 +0200
@@ -0,0 +1,121 @@
+/*-------------------------------------------------------------------------
+ *
+ * lmgrtimeout.c
+ *	routines to manage lock manager timeout sources
+ *
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *	src/backend/storage/lmgr/lmgrtimeout.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "storage/lmgrtimeout.h"
+
+/*
+ * Linked set of lock manager timeout functions
+ */
+static LmgrTimeoutFunctions *CurrentLmgrTimeoutFunctions = NULL;
+
+/*
+ * Register a set of functions for an lock manager timeout
+ */
+void
+RegisterLmgrTimeout(LmgrTimeoutFunctions *functions)
+{
+	Assert(functions != NULL);
+	Assert(functions->enable != NULL);
+	Assert(functions->disable != NULL);
+	Assert(functions->condition != NULL);
+	Assert(functions->reporter != NULL);
+
+	functions->next = CurrentLmgrTimeoutFunctions;
+	CurrentLmgrTimeoutFunctions = functions;
+}
+
+/*
+ * Enable registered lock manager timeouts
+ */
+void
+EnableLmgrTimeouts(void)
+{
+	LmgrTimeoutFunctions *ptr = CurrentLmgrTimeoutFunctions;
+
+	while (ptr)
+	{
+		ptr->enable();
+		ptr = ptr->next;
+	}
+}
+
+/*
+ * Disable registered lock manager timeouts
+ */
+void
+DisableLmgrTimeouts(void)
+{
+	LmgrTimeoutFunctions *ptr = CurrentLmgrTimeoutFunctions;
+
+	while (ptr)
+	{
+		ptr->disable();
+		ptr = ptr->next;
+	}
+}
+
+/*
+ * Call the timeout indicator functions in the order of registration
+ */
+static bool
+DoLmgrTimeoutCondition(LmgrTimeoutFunctions *ptr)
+{
+	if (ptr)
+	{
+		if (ptr->next)
+			if (DoLmgrTimeoutCondition(ptr->next))
+				return true;
+		if (ptr->condition())
+			return true;
+	}
+
+	return false;
+}
+
+/*
+ * Call the timeout indicator and return if any one of them is set
+ */
+bool
+LmgrTimeoutCondition(void)
+{
+	return DoLmgrTimeoutCondition(CurrentLmgrTimeoutFunctions);
+}
+
+/*
+ * Call the timeout error reporter functions in the order of registration
+ */
+static void
+DoReportLmgrTimeoutError(LmgrTimeoutFunctions *ptr)
+{
+	if (ptr)
+	{
+		if (ptr->next)
+			DoReportLmgrTimeoutError(ptr->next);
+
+		/* This may not return because of calling ereport() */
+		ptr->reporter();
+	}
+}
+
+/*
+ * Call the timeout error reporter if set
+ */
+void
+ReportLmgrTimeoutError(void)
+{
+	DoReportLmgrTimeoutError(CurrentLmgrTimeoutFunctions);
+}
diff -durpN postgresql/src/backend/storage/lmgr/lock.c postgresql.1/src/backend/storage/lmgr/lock.c
--- postgresql/src/backend/storage/lmgr/lock.c	2012-06-26 09:10:21.280759421 +0200
+++ postgresql.1/src/backend/storage/lmgr/lock.c	2012-08-21 16:01:33.095488150 +0200
@@ -38,6 +38,7 @@
 #include "miscadmin.h"
 #include "pg_trace.h"
 #include "pgstat.h"
+#include "storage/lmgrtimeout.h"
 #include "storage/proc.h"
 #include "storage/sinvaladt.h"
 #include "storage/spin.h"
@@ -1512,9 +1513,9 @@ WaitOnLock(LOCALLOCK *locallock, Resourc
 
 			/*
 			 * Now that we aren't holding the partition lock, we can give an
-			 * error report including details about the detected deadlock.
+			 * error report on the timeout condition that triggered.
 			 */
-			DeadLockReport();
+			ReportLmgrTimeoutError();
 			/* not reached */
 		}
 	}
diff -durpN postgresql/src/backend/storage/lmgr/lwlock.c postgresql.1/src/backend/storage/lmgr/lwlock.c
--- postgresql/src/backend/storage/lmgr/lwlock.c	2012-06-27 07:41:33.603316276 +0200
+++ postgresql.1/src/backend/storage/lmgr/lwlock.c	2012-08-21 15:54:39.410667067 +0200
@@ -477,7 +477,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
 		for (;;)
 		{
 			/* "false" means cannot accept cancel/die interrupt here. */
-			PGSemaphoreLock(&proc->sem, false);
+			PGSemaphoreLock(&proc->sem, false, NULL);
 			if (!proc->lwWaiting)
 				break;
 			extraWaits++;
@@ -682,7 +682,7 @@ LWLockAcquireOrWait(LWLockId lockid, LWL
 		for (;;)
 		{
 			/* "false" means cannot accept cancel/die interrupt here. */
-			PGSemaphoreLock(&proc->sem, false);
+			PGSemaphoreLock(&proc->sem, false, NULL);
 			if (!proc->lwWaiting)
 				break;
 			extraWaits++;
diff -durpN postgresql/src/backend/storage/lmgr/Makefile postgresql.1/src/backend/storage/lmgr/Makefile
--- postgresql/src/backend/storage/lmgr/Makefile	2012-04-16 19:57:22.458915722 +0200
+++ postgresql.1/src/backend/storage/lmgr/Makefile	2012-08-21 13:05:00.254861812 +0200
@@ -12,7 +12,7 @@ subdir = src/backend/storage/lmgr
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = lmgr.o lock.o proc.o deadlock.o lwlock.o spin.o s_lock.o predicate.o
+OBJS = lmgr.o lmgrtimeout.o lock.o proc.o deadlock.o lwlock.o spin.o s_lock.o predicate.o
 
 include $(top_srcdir)/src/backend/common.mk
 
diff -durpN postgresql/src/backend/storage/lmgr/proc.c postgresql.1/src/backend/storage/lmgr/proc.c
--- postgresql/src/backend/storage/lmgr/proc.c	2012-07-27 10:19:36.933331515 +0200
+++ postgresql.1/src/backend/storage/lmgr/proc.c	2012-08-21 15:56:36.948469452 +0200
@@ -43,6 +43,7 @@
 #include "replication/syncrep.h"
 #include "storage/ipc.h"
 #include "storage/lmgr.h"
+#include "storage/lmgrtimeout.h"
 #include "storage/pmsignal.h"
 #include "storage/proc.h"
 #include "storage/procarray.h"
@@ -640,8 +641,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 lock manager timers if they are still running
+	 * (see ProcSleep)
+	 */
+	DisableLmgrTimeouts();
 
 	/* Unlink myself from the wait queue, if on it (might not be anymore!) */
 	partitionLock = LockHashPartitionLock(lockAwaited->hashcode);
@@ -1028,15 +1032,9 @@ ProcSleep(LOCALLOCK *locallock, LockMeth
 	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.
-	 *
-	 * By delaying the check until we've waited for a bit, we can avoid
-	 * running the rather expensive deadlock-check code in most cases.
+	 * Set the timer for registered lock manager timeouts
 	 */
-	enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
+	EnableLmgrTimeouts();
 
 	/*
 	 * If someone wakes us between LWLockRelease and PGSemaphoreLock,
@@ -1057,7 +1055,7 @@ ProcSleep(LOCALLOCK *locallock, LockMeth
 	 */
 	do
 	{
-		PGSemaphoreLock(&MyProc->sem, true);
+		PGSemaphoreLock(&MyProc->sem, true, LmgrTimeoutCondition);
 
 		/*
 		 * waitStatus could change from STATUS_WAITING to something else
@@ -1203,9 +1201,9 @@ ProcSleep(LOCALLOCK *locallock, LockMeth
 	} 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);
+	DisableLmgrTimeouts();
 
 	/*
 	 * Re-acquire the lock table's partition lock.  We have to do this to hold
@@ -1451,6 +1449,19 @@ check_done:
 
 
 /*
+ * DeadLockTimeoutCondition
+ *	Indicate whether deadlock was detected
+ */
+bool
+DeadLockTimeoutCondition(void)
+{
+	if (deadlock_state == DS_NOT_YET_CHECKED)
+		return false;
+	return get_timeout_indicator(DEADLOCK_TIMEOUT) && (deadlock_state != DS_NO_DEADLOCK);
+}
+
+
+/*
  * ProcWaitForSignal - wait for a signal from another backend.
  *
  * This can share the semaphore normally used for waiting for locks,
@@ -1464,7 +1475,7 @@ check_done:
 void
 ProcWaitForSignal(void)
 {
-	PGSemaphoreLock(&MyProc->sem, true);
+	PGSemaphoreLock(&MyProc->sem, true, NULL);
 }
 
 /*
diff -durpN postgresql/src/backend/utils/init/postinit.c postgresql.1/src/backend/utils/init/postinit.c
--- postgresql/src/backend/utils/init/postinit.c	2012-07-22 16:48:48.525857751 +0200
+++ postgresql.1/src/backend/utils/init/postinit.c	2012-08-21 16:14:31.026731931 +0200
@@ -41,6 +41,7 @@
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/lmgr.h"
+#include "storage/lmgrtimeout.h"
 #include "storage/procarray.h"
 #include "storage/procsignal.h"
 #include "storage/proc.h"
@@ -65,11 +66,17 @@ static void PerformAuthentication(Port *
 static void CheckMyDatabase(const char *name, bool am_superuser);
 static void InitCommunication(void);
 static void ShutdownPostgres(int code, Datum arg);
+static void EnableDeadLockTimeout(void);
+static void DisableDeadLockTimeout(void);
 static void StatementTimeoutHandler(void);
 static bool ThereIsAtLeastOneRole(void);
 static void process_startup_options(Port *port, bool am_superuser);
 static void process_settings(Oid databaseid, Oid roleid);
 
+static LmgrTimeoutFunctions DeadLockTimeoutFunctions = {
+	EnableDeadLockTimeout, DisableDeadLockTimeout,
+	DeadLockTimeoutCondition, DeadLockReport
+};
 
 /*** InitPostgres support ***/
 
@@ -501,6 +508,7 @@ InitPostgres(const char *in_dbname, Oid
 	if (!bootstrap)
 	{
 		RegisterTimeout(DEADLOCK_TIMEOUT, CheckDeadLock);
+		RegisterLmgrTimeout(&DeadLockTimeoutFunctions);
 		RegisterTimeout(STATEMENT_TIMEOUT, StatementTimeoutHandler);
 	}
 
@@ -982,6 +990,26 @@ ShutdownPostgres(int code, Datum arg)
 	LockReleaseAll(USER_LOCKMETHOD, true);
 }
 
+static void
+EnableDeadLockTimeout(void)
+{
+	/*
+	 * 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.
+	 *
+	 * By delaying the check until we've waited for a bit, we can avoid
+	 * running the rather expensive deadlock-check code in most cases.
+	 */
+	enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
+}
+
+static void
+DisableDeadLockTimeout(void)
+{
+	disable_timeout(DEADLOCK_TIMEOUT, true);
+}
 
 /*
  * STATEMENT_TIMEOUT handler: trigger a query-cancel interrupt.
diff -durpN postgresql/src/include/storage/lmgrtimeout.h postgresql.1/src/include/storage/lmgrtimeout.h
--- postgresql/src/include/storage/lmgrtimeout.h	1970-01-01 01:00:00.000000000 +0100
+++ postgresql.1/src/include/storage/lmgrtimeout.h	2012-08-21 13:30:43.050291053 +0200
@@ -0,0 +1,36 @@
+/*-------------------------------------------------------------------------
+ *
+ * lmgrtimeout.h
+ *	  POSTGRES lock manager timeout definitions.
+ *
+ *
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/storage/lmgrtimeout.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef LMGRTIMEOUT_H
+#define LMGRTIMEOUT_H
+
+/* callback function signatures for lock manager timeouts */
+typedef void (*LmgrTimeoutCallback)(void);
+typedef bool (*LmgrTimeoutConditionCallback)(void);
+
+typedef struct LmgrTimeoutFunctions {
+	LmgrTimeoutCallback		enable;
+	LmgrTimeoutCallback		disable;
+	LmgrTimeoutConditionCallback	condition;
+	LmgrTimeoutCallback		reporter;
+	struct LmgrTimeoutFunctions *next;
+} LmgrTimeoutFunctions;
+
+/* register and handle lock manager timeouts */
+extern void RegisterLmgrTimeout(LmgrTimeoutFunctions *functions);
+extern void EnableLmgrTimeouts(void);
+extern void DisableLmgrTimeouts(void);
+extern bool LmgrTimeoutCondition(void);
+extern void ReportLmgrTimeoutError(void);
+
+#endif	/* LMGRTIMEOUT_H */
diff -durpN postgresql/src/include/storage/pg_sema.h postgresql.1/src/include/storage/pg_sema.h
--- postgresql/src/include/storage/pg_sema.h	2012-04-16 19:57:22.672918205 +0200
+++ postgresql.1/src/include/storage/pg_sema.h	2012-08-21 15:44:06.655459333 +0200
@@ -62,6 +62,8 @@ typedef HANDLE PGSemaphoreData;
 typedef PGSemaphoreData *PGSemaphore;
 
 
+typedef bool (*PGSemaphoreCondition)(void);
+
 /* Module initialization (called during postmaster start or shmem reinit) */
 extern void PGReserveSemaphores(int maxSemas, int port);
 
@@ -72,7 +74,8 @@ extern void PGSemaphoreCreate(PGSemaphor
 extern void PGSemaphoreReset(PGSemaphore sema);
 
 /* Lock a semaphore (decrement count), blocking if count would be < 0 */
-extern void PGSemaphoreLock(PGSemaphore sema, bool interruptOK);
+extern void PGSemaphoreLock(PGSemaphore sema, bool interruptOK,
+						PGSemaphoreCondition condition);
 
 /* Unlock a semaphore (increment count) */
 extern void PGSemaphoreUnlock(PGSemaphore sema);
diff -durpN postgresql/src/include/storage/proc.h postgresql.1/src/include/storage/proc.h
--- postgresql/src/include/storage/proc.h	2012-07-22 16:48:48.548857900 +0200
+++ postgresql.1/src/include/storage/proc.h	2012-08-21 15:10:38.641347523 +0200
@@ -244,6 +244,7 @@ extern int	ProcSleep(LOCALLOCK *localloc
 extern PGPROC *ProcWakeup(PGPROC *proc, int waitStatus);
 extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock);
 extern void CheckDeadLock(void);
+extern bool DeadLockTimeoutCondition(void);
 extern bool IsWaitingForLock(void);
 extern void LockErrorCleanup(void);
 
diff -durpN postgresql.1/doc/src/sgml/config.sgml postgresql.2/doc/src/sgml/config.sgml
--- postgresql.1/doc/src/sgml/config.sgml	2012-08-12 19:05:58.684948018 +0200
+++ postgresql.2/doc/src/sgml/config.sgml	2012-08-21 14:13:19.818572888 +0200
@@ -4965,7 +4965,10 @@ COPY postgres_log FROM '/full/path/to/lo
         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>
@@ -4976,6 +4979,60 @@ COPY postgres_log FROM '/full/path/to/lo
       </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 tries to acquire a heavy-weight lock on rows,
+        pages, tables, indices or other objects and the lock(s) has to wait
+        more than the specified number of milliseconds. 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-lock-timeout-option" xreflabel="lock_timeout_option">
+      <term><varname>lock_timeout_option</varname> (<type>enum</type>)</term>
+      <indexterm>
+       <primary><varname>lock_timeout_option</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Control the behaviour of <varname>lock_timeout</>. Possible values are
+        'per_lock' and 'per_statement'.
+       </para>
+
+       <para>
+        With 'per_lock' in effect and if the statement involves more than one
+        lock, the timeout applies to every one of them individually, starting
+        from the time the server attempts to lock an object. This makes the
+        statement possibly wait for up to N * <varname>lock_timeout</> time in
+        the worst case where N is the number of locks attempted. This is the
+        default.
+       </para>
+
+       <para>
+        With 'per_statement' in effect, <varname>lock_timeout</> behaves like
+        <varname>statement_timeout</>: the specified timeout applies to all
+        the locks in a cumulative way, starting from the time the command
+        arrives at the server from the client.
+       </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 -durpN postgresql.1/doc/src/sgml/ref/lock.sgml postgresql.2/doc/src/sgml/ref/lock.sgml
--- postgresql.1/doc/src/sgml/ref/lock.sgml	2012-04-16 19:57:22.229913063 +0200
+++ postgresql.2/doc/src/sgml/ref/lock.sgml	2012-08-21 14:13:19.819572894 +0200
@@ -39,8 +39,11 @@ LOCK [ TABLE ] [ ONLY ] <replaceable cla
    <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
+   error is emitted. If <varname>lock_timeout</varname> is set to a value
+   higher than 0, and the lock cannot be acquired under the specified
+   timeout value in milliseconds, 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>
diff -durpN postgresql.1/doc/src/sgml/ref/select.sgml postgresql.2/doc/src/sgml/ref/select.sgml
--- postgresql.1/doc/src/sgml/ref/select.sgml	2012-08-08 06:26:48.832119566 +0200
+++ postgresql.2/doc/src/sgml/ref/select.sgml	2012-08-21 14:13:19.821572908 +0200
@@ -1241,6 +1241,14 @@ FOR SHARE [ OF <replaceable class="param
    </para>
 
    <para>
+    If <literal>NOWAIT</> option is not specified and <varname>lock_timeout</varname>
+    is set to a value higher than 0, and the lock or statement (depending on
+    <varname>lock_timeout_option</varname>) needs to wait more than
+    the specified value in milliseconds, the command reports an error after
+    timing out, rather than waiting indefinitely.
+   </para>
+
+   <para>
     If specific tables are named in <literal>FOR UPDATE</literal>
     or <literal>FOR SHARE</literal>,
     then only rows coming from those tables are locked; any other
diff -durpN postgresql.1/src/backend/storage/lmgr/proc.c postgresql.2/src/backend/storage/lmgr/proc.c
--- postgresql.1/src/backend/storage/lmgr/proc.c	2012-08-21 15:56:36.948469452 +0200
+++ postgresql.2/src/backend/storage/lmgr/proc.c	2012-08-21 16:08:02.153133960 +0200
@@ -56,6 +56,8 @@
 /* GUC variables */
 int			DeadlockTimeout = 1000;
 int			StatementTimeout = 0;
+int			LockTimeout = 0;
+int			LockTimeoutOption = LOCK_TIMEOUT_PER_LOCK;
 bool		log_lock_waits = false;
 
 /* Pointer to this process's PGPROC and PGXACT structs, if any */
diff -durpN postgresql.1/src/backend/utils/init/postinit.c postgresql.2/src/backend/utils/init/postinit.c
--- postgresql.1/src/backend/utils/init/postinit.c	2012-08-21 16:14:31.026731931 +0200
+++ postgresql.2/src/backend/utils/init/postinit.c	2012-08-21 16:14:05.715565301 +0200
@@ -57,6 +57,7 @@
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/timeout.h"
+#include "utils/timestamp.h"
 #include "utils/tqual.h"
 
 
@@ -68,6 +69,11 @@ static void InitCommunication(void);
 static void ShutdownPostgres(int code, Datum arg);
 static void EnableDeadLockTimeout(void);
 static void DisableDeadLockTimeout(void);
+static void LockTimeoutHandler(void);
+static void EnableLockTimeout(void);
+static void DisableLockTimeout(void);
+static bool LockTimeoutCondition(void);
+static void LockTimeoutError(void);
 static void StatementTimeoutHandler(void);
 static bool ThereIsAtLeastOneRole(void);
 static void process_startup_options(Port *port, bool am_superuser);
@@ -78,6 +84,11 @@ static LmgrTimeoutFunctions DeadLockTime
 	DeadLockTimeoutCondition, DeadLockReport
 };
 
+static LmgrTimeoutFunctions LockTimeoutFunctions = {
+	EnableLockTimeout, DisableLockTimeout,
+	LockTimeoutCondition, LockTimeoutError
+};
+
 /*** InitPostgres support ***/
 
 
@@ -509,6 +520,8 @@ InitPostgres(const char *in_dbname, Oid
 	{
 		RegisterTimeout(DEADLOCK_TIMEOUT, CheckDeadLock);
 		RegisterLmgrTimeout(&DeadLockTimeoutFunctions);
+		RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler);
+		RegisterLmgrTimeout(&LockTimeoutFunctions);
 		RegisterTimeout(STATEMENT_TIMEOUT, StatementTimeoutHandler);
 	}
 
@@ -1012,6 +1025,84 @@ DisableDeadLockTimeout(void)
 }
 
 /*
+ * LOCK_TIMEOUT handler: disable all subsequent timeouts
+ *
+ * This ensures that the error coming from lock timeout is reported
+ * instead of triggering the query-cancel interrupt (and reporting
+ * the statement timeout error) in case they trigger at the same time.
+ *
+ * Also, we must remove ourselves from the SHM wait queue just like
+ * DeadLockCheck() does so ProcSleep() can return and the error can
+ * be reported.
+ */
+static void
+LockTimeoutHandler(void)
+{
+	Assert(MyProc->waitLock != NULL);
+
+	RemoveFromWaitQueue(MyProc, LockTagHashCode(&(MyProc->waitLock->tag)));
+
+	disable_all_timeouts(true);
+}
+ 
+/*
+ * Enable LOCK_TIMEOUT
+ */
+static void
+EnableLockTimeout(void)
+{
+	if (LockTimeout > 0)
+	{
+		switch (LockTimeoutOption)
+		{
+			case LOCK_TIMEOUT_PER_LOCK:
+				enable_timeout_after(LOCK_TIMEOUT, LockTimeout);
+				break;
+			case LOCK_TIMEOUT_PER_STMT:
+				enable_timeout_at(LOCK_TIMEOUT,
+							TimestampTzPlusMilliseconds(
+									GetCurrentStatementStartTimestamp(),
+									LockTimeout));
+				break;
+			default:
+				elog(ERROR, "unhandled lock_timeout_option value: \"%s\"",
+							GetConfigOptionByName("lock_timeout_option", NULL));
+				break;
+		}
+	}
+}
+
+/*
+ * Disable LOCK_TIMEOUT
+ */
+static void
+DisableLockTimeout(void)
+{
+	disable_timeout(LOCK_TIMEOUT, true);
+}
+
+/*
+ * Indicator for LOCK_TIMEOUT.
+ */
+static bool
+LockTimeoutCondition(void)
+{
+	return get_timeout_indicator(LOCK_TIMEOUT);
+}
+
+/*
+ * Error reporting for LOCK_TIMEOUT
+ */
+static void
+LockTimeoutError(void)
+{
+	if (get_timeout_indicator(LOCK_TIMEOUT))
+		ereport(ERROR,
+				(errcode(ERRCODE_QUERY_CANCELED),
+				 errmsg("canceling statement due to lock timeout")));
+}
+
+/*
  * STATEMENT_TIMEOUT handler: trigger a query-cancel interrupt.
  */
 static void
diff -durpN postgresql.1/src/backend/utils/misc/guc.c postgresql.2/src/backend/utils/misc/guc.c
--- postgresql.1/src/backend/utils/misc/guc.c	2012-08-16 10:42:50.226582077 +0200
+++ postgresql.2/src/backend/utils/misc/guc.c	2012-08-21 14:13:19.824572927 +0200
@@ -389,6 +389,17 @@ static const struct config_enum_entry sy
 };
 
 /*
+ * Control behaviour of lock_timeout:
+ * - timeout applied per lock from the time the lock is attempted to be taken
+ * - timeout applied per statement from the time the statements has started
+ */
+static const struct config_enum_entry lock_timeout_options[] = {
+	{"per_lock", LOCK_TIMEOUT_PER_LOCK, false},
+	{"per_statement", LOCK_TIMEOUT_PER_STMT, false},
+	{NULL, 0, false}
+};
+
+/*
  * Options for enum values stored in other modules
  */
 extern const struct config_enum_entry wal_level_options[];
@@ -1861,6 +1872,17 @@ static struct config_int ConfigureNamesI
 	},
 
 	{
+		{"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
@@ -3146,6 +3168,16 @@ static struct config_enum ConfigureNames
 		NULL, NULL, NULL
 	},
 
+	{
+		{"lock_timeout_option", PGC_USERSET, CLIENT_CONN_STATEMENT,
+			gettext_noop("Sets the lock_timeout behaviour."),
+			NULL
+		},
+		&LockTimeoutOption,
+		LOCK_TIMEOUT_PER_LOCK, lock_timeout_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"log_error_verbosity", PGC_SUSET, LOGGING_WHAT,
 			gettext_noop("Sets the verbosity of logged messages."),
diff -durpN postgresql.1/src/backend/utils/misc/postgresql.conf.sample postgresql.2/src/backend/utils/misc/postgresql.conf.sample
--- postgresql.1/src/backend/utils/misc/postgresql.conf.sample	2012-08-16 10:42:50.227582083 +0200
+++ postgresql.2/src/backend/utils/misc/postgresql.conf.sample	2012-08-21 14:13:19.825572933 +0200
@@ -529,6 +529,11 @@
 #------------------------------------------------------------------------------
 
 #deadlock_timeout = 1s
+#lock_timeout = 0			# timeout value for heavy-weight locks
+					# taken by statements. 0 disables timeout
+					# unit in milliseconds, default is 0
+#lock_timeout_option = 'per_lock'	# behaviour of lock_timeout. possible
+					# values are: 'per_lock' or 'per_statement'
 #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 -durpN postgresql.1/src/include/storage/proc.h postgresql.2/src/include/storage/proc.h
--- postgresql.1/src/include/storage/proc.h	2012-08-21 15:10:38.641347523 +0200
+++ postgresql.2/src/include/storage/proc.h	2012-08-21 15:00:48.765532912 +0200
@@ -219,8 +219,15 @@ extern PGPROC *PreparedXactProcs;
 /* configurable options */
 extern int	DeadlockTimeout;
 extern int	StatementTimeout;
+extern int	LockTimeout;
+extern int	LockTimeoutOption;
 extern bool log_lock_waits;
 
+typedef enum LockTimeoutOptions {
+	LOCK_TIMEOUT_PER_LOCK,
+	LOCK_TIMEOUT_PER_STMT
+} LockTimeoutOptions;
+
 
 /*
  * Function Prototypes
diff -durpN postgresql.1/src/include/utils/timeout.h postgresql.2/src/include/utils/timeout.h
--- postgresql.1/src/include/utils/timeout.h	2012-07-22 16:48:48.549857907 +0200
+++ postgresql.2/src/include/utils/timeout.h	2012-08-21 14:13:19.825572933 +0200
@@ -25,6 +25,7 @@ typedef enum TimeoutId
 	/* Predefined timeout reasons */
 	STARTUP_PACKET_TIMEOUT,
 	DEADLOCK_TIMEOUT,
+	LOCK_TIMEOUT,
 	STATEMENT_TIMEOUT,
 	STANDBY_DEADLOCK_TIMEOUT,
 	STANDBY_TIMEOUT,
diff -durpN postgresql.1/src/test/regress/expected/prepared_xacts.out postgresql.2/src/test/regress/expected/prepared_xacts.out
--- postgresql.1/src/test/regress/expected/prepared_xacts.out	2012-04-16 19:57:22.776919413 +0200
+++ postgresql.2/src/test/regress/expected/prepared_xacts.out	2012-08-21 14:13:19.825572933 +0200
@@ -198,6 +198,22 @@ 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;
+-- Test lock_timeout_option = 'per_statement' and see that lock_timeout
+-- triggers instead of statement_timeout if both are set.
+-- pxtest3 should be locked because of the pending DROP
+set statement_timeout to 2000;
+set lock_timeout to 2000;
+set lock_timeout_option to 'per_statement';
+SELECT * FROM pxtest3;
+ERROR:  canceling statement due to lock timeout
+reset lock_timeout;
+reset statement_timeout;
+reset lock_timeout_option;
 -- Disconnect, we will continue testing in a different backend
 \c -
 -- There should still be two prepared transactions
@@ -213,6 +229,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 -durpN postgresql.1/src/test/regress/sql/prepared_xacts.sql postgresql.2/src/test/regress/sql/prepared_xacts.sql
--- postgresql.1/src/test/regress/sql/prepared_xacts.sql	2012-04-16 19:57:22.796919644 +0200
+++ postgresql.2/src/test/regress/sql/prepared_xacts.sql	2012-08-21 14:13:19.826572940 +0200
@@ -126,6 +126,22 @@ 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;
+
+-- Test lock_timeout_option = 'per_statement' and see that lock_timeout
+-- triggers instead of statement_timeout if both are set.
+-- pxtest3 should be locked because of the pending DROP
+set statement_timeout to 2000;
+set lock_timeout to 2000;
+set lock_timeout_option to 'per_statement';
+SELECT * FROM pxtest3;
+reset lock_timeout;
+reset statement_timeout;
+reset lock_timeout_option;
+
 -- Disconnect, we will continue testing in a different backend
 \c -
 
@@ -137,6 +153,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

Reply via email to