Waiting relation in GDD should consider lock conflict map.

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:
+