Special case in ProcSleep() wasn't sufficiently general: must check to

see if we shouldn't block whenever we insert ourselves anywhere before
the end of the queue, not only at the front.

Author: tglsfdc

src/backend/storage/lmgr/README

@@ -1,4 +1,4 @@
-$Header: /cvsroot/pgsql/src/backend/storage/lmgr/README,v 1.7 2001/01/25 03:31:16 tgl Exp $
+$Header: /cvsroot/pgsql/src/backend/storage/lmgr/README,v 1.8 2001/01/26 18:23:12 tgl Exp $
 
 There are two fundamental lock structures: the per-lockable-object LOCK
 struct, and the per-lock-holder HOLDER struct.  A LOCK object exists
@@ -178,11 +178,10 @@ request of any pending waiter, then the process will be inserted in the
 wait queue just ahead of the first such waiter.  (If we did not make this
 check, the deadlock detection code would adjust the queue order to resolve
 the conflict, but it's relatively cheap to make the check in ProcSleep and
-avoid a deadlock timeout delay in this case.)  Note special case: if the
-process holds locks that conflict with the first waiter, so that it would
-go at the front of the queue, and its request does not conflict with the
-already-granted locks, then the process will be granted the lock without
-going to sleep at all.
+avoid a deadlock timeout delay in this case.)  Note special case when
+inserting before the end of the queue: if the process's request does not
+conflict with any existing lock nor any waiting request before its insertion
+point, then go ahead and grant the lock without waiting.
 
 When a lock is released, the lock release routine (ProcLockWakeup) scans
 the lock object's wait queue.  Each waiter is awoken if (a) its request
@@ -192,7 +191,7 @@ ensures that conflicting requests are granted in order of arrival.
 There are cases where a later waiter must be allowed to go in front of
 conflicting earlier waiters to avoid deadlock, but it is not
 ProcLockWakeup's responsibility to recognize these cases; instead, the
-deadlock detection code re-orders the wait queue when necessary.
+deadlock detection code will re-order the wait queue when necessary.
 
 To perform deadlock checking, we use the standard method of viewing the
 various processes as nodes in a directed graph (the waits-for graph or
@@ -263,13 +262,13 @@ orders) as well as the current real order.
 
 The easiest way to handle this seems to be to have a lookaside table that
 shows the proposed new queue order for each wait queue that we are
-considering rearranging.  This table is passed to FindLockCycle, and it
-believes the given queue order rather than the "real" order for each lock
+considering rearranging.  This table is checked by FindLockCycle, and it
+believes the proposed queue order rather than the real order for each lock
 that has an entry in the lookaside table.
 
 We build a proposed new queue order by doing a "topological sort" of the
 existing entries.  Each soft edge that we are currently considering
-reversing is a property of the partial order that the topological sort
+reversing creates a property of the partial order that the topological sort
 has to enforce.  We must use a sort method that preserves the input
 ordering as much as possible, so as not to gratuituously break arrival
 order for processes not involved in a deadlock.  (This is not true of the

src/backend/storage/lmgr/proc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.97 2001/01/25 03:31:16 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v 1.98 2001/01/26 18:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -537,13 +537,18 @@ ProcSleep(LOCKMETHODTABLE *lockMethodTable,
 	 * me to before that waiter anyway; but it's relatively cheap to detect
 	 * such a conflict immediately, and avoid delaying till deadlock timeout.
 	 *
-	 * Special case: if I find I should go in front of the first waiter,
-	 * and I do not conflict with already-held locks, then just grant myself
-	 * the requested lock immediately.
+	 * Special case: if I find I should go in front of some waiter, check
+	 * to see if I conflict with already-held locks or the requests before
+	 * that waiter.  If not, then just grant myself the requested lock
+	 * immediately.  This is the same as the test for immediate grant in
+	 * LockAcquire, except we are only considering the part of the wait queue
+	 * before my insertion point.
 	 * ----------------------
 	 */
 	if (myHeldLocks != 0)
 	{
+		int			aheadRequests = 0;
+
 		proc = (PROC *) MAKE_PTR(waitQueue->links.next);
 		for (i = 0; i < waitQueue->size; i++)
 		{
@@ -557,26 +562,30 @@ ProcSleep(LOCKMETHODTABLE *lockMethodTable,
 					MyProc->errType = STATUS_ERROR;
 					return STATUS_ERROR;
 				}
-				if (i == 0)
+				/* I must go before this waiter.  Check special case. */
+				if ((lockctl->conflictTab[lockmode] & aheadRequests) == 0 &&
+					LockCheckConflicts(lockMethodTable,
+									   lockmode,
+									   lock,
+									   holder,
+									   MyProc,
+									   NULL) == STATUS_OK)
 				{
-					/* I must go before first waiter.  Check special case. */
-					if (LockCheckConflicts(lockMethodTable,
-										   lockmode,
-										   lock,
-										   holder,
-										   MyProc,
-										   NULL) == STATUS_OK)
-					{
-						/* Skip the wait and just grant myself the lock. */
-						GrantLock(lock, holder, lockmode);
-						return STATUS_OK;
-					}
+					/* Skip the wait and just grant myself the lock. */
+					GrantLock(lock, holder, lockmode);
+					return STATUS_OK;
 				}
 				/* Break out of loop to put myself before him */
 				break;
 			}
+			/* Nope, so advance to next waiter */
+			aheadRequests |= (1 << proc->waitLockMode);
 			proc = (PROC *) MAKE_PTR(proc->links.next);
 		}
+		/*
+		 * If we fall out of loop normally, proc points to waitQueue head,
+		 * so we will insert at tail of queue as desired.
+		 */
 	}
 	else
 	{
@@ -739,7 +748,7 @@ ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock)
 	PROC_QUEUE *waitQueue = &(lock->waitProcs);
 	int			queue_size = waitQueue->size;
 	PROC	   *proc;
-	int			conflictMask = 0;
+	int			aheadRequests = 0;
 
 	Assert(queue_size >= 0);
 
@@ -756,7 +765,7 @@ ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock)
 		 * Waken if (a) doesn't conflict with requests of earlier waiters,
 		 * and (b) doesn't conflict with already-held locks.
 		 */
-		if (((1 << lockmode) & conflictMask) == 0 &&
+		if ((lockctl->conflictTab[lockmode] & aheadRequests) == 0 &&
 			LockCheckConflicts(lockMethodTable,
 							   lockmode,
 							   lock,
@@ -775,8 +784,8 @@ ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock)
 		}
 		else
 		{
-			/* Cannot wake this guy.  Add his request to conflict mask. */
-			conflictMask |= lockctl->conflictTab[lockmode];
+			/* Cannot wake this guy. Remember his request for later checks. */
+			aheadRequests |= (1 << lockmode);
 			proc = (PROC *) MAKE_PTR(proc->links.next);
 		}
 	}