Waiting relation in GDD should consider lock conflict map. (#7624)

When determine waiting relation in global dead lock detector,
we should also check whether the waiter's lockmode conflict with
holder's holdmask.
Co-authored-by: NNing Yu <nyu@pivotal.io>
Reviewed-by: NZhenghua Lyu <zlv@pivotal.io>

src/backend/storage/lmgr/lock.c

@@ -4609,3 +4609,21 @@ setFPHoldTillEndXact(Oid relid)
 
 	return result;
 }
+
+/*
+ * Check whether a waiter's request lockmode conflict with
+ * the holder's hold mask
+ */
+bool
+CheckWaitLockModeConflictHoldMask(LOCKTAG tag, LOCKMODE waitLockMode, LOCKMASK holderMask)
+{
+	int			waiterConflictMask;
+	LOCKMETHODID lockmethodid = (LOCKMETHODID) tag.locktag_lockmethodid;
+
+	Assert(0 < lockmethodid && lockmethodid < lengthof(LockMethods));
+
+	waiterConflictMask = LockMethods[lockmethodid]->conflictTab[waitLockMode];
+	if (holderMask & waiterConflictMask)
+		return true;
+	return false;
+}

src/backend/utils/gdd/gddfuncs.c

@@ -18,6 +18,7 @@
 #include "cdb/cdbvars.h"
 #include "funcapi.h"
 #include "libpq-fe.h"
+#include "storage/lock.h"
 #include "storage/proc.h"
 #include "utils/builtins.h"
 
@@ -187,6 +188,7 @@ pg_dist_wait_status(PG_FUNCTION_ARGS)
 	 * A relation is that:
 	 * (waiter.granted == false &&
 	 *  holder.granted == true &&
+	 *  waiter.waitLockMode conflict with holder.holdMask &&
 	 *  waiter.pid != holder.pid &&
 	 *  waiter.locktype == holder.locktype &&
 	 *  waiter.locktags == holder.locktags)
@@ -233,6 +235,13 @@ pg_dist_wait_status(PG_FUNCTION_ARGS)
 				continue;
 			if (w_lock->pid == h_lock->pid)
 				continue;
+			/* If waiter and holder have different lock methods, they must not be conflict */
+			if (w_lock->locktag.locktag_lockmethodid != h_lock->locktag.locktag_lockmethodid)
+				continue;
+			/* If waiter and holder are not conflict, should skip this edge */
+			if (!CheckWaitLockModeConflictHoldMask(w_lock->locktag,
+					w_lock->waitLockMode, h_lock->holdMask))
+				continue;
 			if (!lockEqual(w_lock, h_lock))
 				continue;
 

src/include/storage/lock.h

@@ -641,4 +641,6 @@ extern void VirtualXactLockTableInsert(VirtualTransactionId vxid);
 extern void VirtualXactLockTableCleanup(void);
 extern bool VirtualXactLock(VirtualTransactionId vxid, bool wait);
 
+/* Check whether a waiter's request lockmode conflict with the holder's hold mask */
+extern bool CheckWaitLockModeConflictHoldMask(LOCKTAG tag, LOCKMODE waitLockMode, LOCKMASK holderMask);
 #endif   /* LOCK_H */

src/test/isolation2/expected/gdd/non-lock-108.out

+DROP TABLE IF EXISTS t108;
+DROP
+CREATE TABLE t108 (id int, val int);
+CREATE
+INSERT INTO t108 (id, val) SELECT i, i FROM generate_series(1, 100) i;
+INSERT 100
+
+10: BEGIN;
+BEGIN
+20: BEGIN;
+BEGIN
+30: BEGIN;
+BEGIN
+
+10: INSERT INTO t108 VALUES(segid(1,1),segid(1,1));
+INSERT 1
+
+20: INSERT INTO t108 VALUES(segid(1,1),segid(1,1));
+INSERT 1
+
+30: INSERT INTO t108 VALUES(segid(1,1),segid(1,1));
+INSERT 1
+
+-- ANALYZE holds ShareUpdateExclusiveLock, they conflict with each other.
+-- But they are not conflict with INSERT, So GDD graph on seg1 should be
+-- 20 -> 10 and 30 -> 10 and no cycle in this case.
+10: ANALYZE t108;
+ANALYZE
+
+20&: ANALYZE t108;  <waiting ...>
+
+30&: ANALYZE t108;  <waiting ...>
+
+SELECT pg_sleep(20);
+ pg_sleep 
+----------
+          
+(1 row)
+
+-- con10/20/30 should finish normally.
+10q: ... <quitting>
+
+20<:  <... completed>
+ANALYZE
+20q: ... <quitting>
+
+30<:  <... completed>
+ANALYZE
+30q: ... <quitting>
+

src/test/isolation2/sql/gdd/non-lock-108.sql

+DROP TABLE IF EXISTS t108;
+CREATE TABLE t108 (id int, val int);
+INSERT INTO t108 (id, val) SELECT i, i FROM generate_series(1, 100) i;
+
+10: BEGIN;
+20: BEGIN;
+30: BEGIN;
+
+10: INSERT INTO t108 VALUES(segid(1,1),segid(1,1));
+
+20: INSERT INTO t108 VALUES(segid(1,1),segid(1,1));
+
+30: INSERT INTO t108 VALUES(segid(1,1),segid(1,1));
+
+-- ANALYZE holds ShareUpdateExclusiveLock, they conflict with each other.
+-- But they are not conflict with INSERT, So GDD graph on seg1 should be
+-- 20 -> 10 and 30 -> 10 and no cycle in this case.
+10: ANALYZE t108;
+
+20&: ANALYZE t108;
+
+30&: ANALYZE t108;
+
+SELECT pg_sleep(20);
+
+-- con10/20/30 should finish normally.
+10q:
+
+20<:
+20q:
+
+30<:
+30q:
+