v8-0003-Fix-CREATE-INDEX-CONCURRENTLY-in-precence-of-vxid.patch

application/octet-stream

Filename: v8-0003-Fix-CREATE-INDEX-CONCURRENTLY-in-precence-of-vxid.patch
Type: application/octet-stream
Part: 2
Message: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v8-0003
Subject: Fix CREATE INDEX CONCURRENTLY in precence of vxids converted to 2pc
File+
src/backend/access/transam/twophase.c 43 3
src/backend/storage/lmgr/lock.c 99 20
src/include/access/twophase.h 13 0
From 9000a4cb36ee6480b2c841edd7f14058d1716099 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Mon, 19 Jul 2021 11:50:02 +0500
Subject: [PATCH v8 3/4] Fix CREATE INDEX CONCURRENTLY in precence of vxids
 converted to 2pc

---
 src/backend/access/transam/twophase.c |  46 +++++++++-
 src/backend/storage/lmgr/lock.c       | 119 +++++++++++++++++++++-----
 src/include/access/twophase.h         |  13 +++
 3 files changed, 155 insertions(+), 23 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index f67d813c56..55c68364ce 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -459,14 +459,15 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
 	proc->pgprocno = gxact->pgprocno;
 	SHMQueueElemInit(&(proc->links));
 	proc->waitStatus = PROC_WAIT_STATUS_OK;
-	/* We set up the gxact's VXID as InvalidBackendId/XID */
-	proc->lxid = (LocalTransactionId) xid;
+	/* We set up the gxact's VXID as real for CIC purposes */
+	proc->lxid = MyProc->lxid;
 	proc->xid = xid;
 	Assert(proc->xmin == InvalidTransactionId);
 	proc->delayChkpt = false;
 	proc->statusFlags = 0;
 	proc->pid = 0;
-	proc->backendId = InvalidBackendId;
+	/* May be backendId of startup process */
+	proc->backendId = MyBackendId;
 	proc->databaseId = databaseid;
 	proc->roleId = owner;
 	proc->tempNamespaceId = InvalidOid;
@@ -846,6 +847,45 @@ TwoPhaseGetGXact(TransactionId xid, bool lock_held)
 	return result;
 }
 
+/*
+ * TwoPhaseGetXidByVXid
+ *		Try to lookup for vxid among prepared xacts
+ */
+XidListEntry
+TwoPhaseGetXidByVXid(VirtualTransactionId vxid)
+{
+	int				i;
+	XidListEntry	result;
+	result.next = NULL;
+	result.xid = InvalidTransactionId;
+
+	LWLockAcquire(TwoPhaseStateLock, LW_SHARED);
+
+	for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
+	{
+		GlobalTransaction gxact = TwoPhaseState->prepXacts[i];
+		PGPROC	   *proc;
+		proc = &ProcGlobal->allProcs[gxact->pgprocno];
+
+		if (proc->backendId == vxid.backendId &&
+				proc->lxid == vxid.localTransactionId)
+		{
+			if (result.xid != InvalidTransactionId)
+			{
+				XidListEntry *copy = palloc(sizeof(XidListEntry));
+				copy->next = result.next;
+				copy->xid = result.xid;
+				result.next = copy;
+			}
+			result.xid = gxact->xid;
+		}
+	}
+
+	LWLockRelease(TwoPhaseStateLock);
+
+	return result;
+}
+
 /*
  * TwoPhaseGetDummyBackendId
  *		Get the dummy backend ID for prepared transaction specified by XID
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 108b4d9023..becddef7fe 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -386,6 +386,7 @@ static void LockRefindAndRelease(LockMethod lockMethodTable, PGPROC *proc,
 								 bool decrement_strong_lock_count);
 static void GetSingleProcBlockerStatusData(PGPROC *blocked_proc,
 										   BlockedProcsData *data);
+static bool PreparedXactLock(VirtualTransactionId vxid, TransactionId xid, bool wait);
 
 
 /*
@@ -3017,7 +3018,14 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
 				/* Conflict! */
 				GET_VXID_FROM_PGPROC(vxid, *proc);
 
-				if (VirtualTransactionIdIsValid(vxid))
+				/* Prefer real Xid over local Xid */
+				if (TransactionIdIsValid(proc->xid))
+				{
+					vxids[count].backendId = InvalidBackendId;
+					vxids[count].localTransactionId = proc->xid;
+					count++;
+				}
+				else if (VirtualTransactionIdIsValid(vxid))
 					vxids[count++] = vxid;
 				/* else, xact already committed or aborted */
 
@@ -3078,7 +3086,14 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
 
 				GET_VXID_FROM_PGPROC(vxid, *proc);
 
-				if (VirtualTransactionIdIsValid(vxid))
+				/* Prefer real Xid over local Xid */
+				if (TransactionIdIsValid(proc->xid))
+				{
+					vxids[count].backendId = InvalidBackendId;
+					vxids[count].localTransactionId = proc->xid;
+					count++;
+				}
+				else if (VirtualTransactionIdIsValid(vxid))
 				{
 					int			i;
 
@@ -4447,6 +4462,74 @@ VirtualXactLockTableCleanup(void)
 	}
 }
 
+/*
+ *		PreparedXactLock
+ *
+ *		Wait for xid completition if have xid. Otherwise try to find xid among
+ *		fake procarray entries.
+ */
+static bool PreparedXactLock(VirtualTransactionId vxid, TransactionId xid, bool wait)
+{
+	LockAcquireResult	lar;
+	LOCKTAG				tag;
+	XidListEntry		xidlist;
+	XidListEntry	   *xidlist_ptr = NULL; /* pointer to TwoPhaseGetXidByVXid()s pallocs */
+	bool				result;
+
+	/* Questionable heuristics */
+	if (max_prepared_xacts == 0)
+		return true;
+
+	if (TransactionIdIsValid(xid))
+	{
+		/* We know exact xid - no need to search in 2PC state */
+		xidlist.xid = xid;
+		xidlist.next = NULL;
+	}
+	else
+	{
+		/* We must search for vxids in 2pc state */
+		/* XXX: O(N*N) complexity where N is number of prepared xacts */
+		xidlist = TwoPhaseGetXidByVXid(vxid);
+		/* Return if transaction is gone entirely */
+		if (!TransactionIdIsValid(xidlist.xid))
+			return true;
+		xidlist_ptr = xidlist.next;
+	}
+
+
+	while (true)
+	{
+		/* Iterate over possible xids */
+		Assert(TransactionIdIsValid(xidlist.xid));
+		SET_LOCKTAG_TRANSACTION(tag, xidlist.xid);
+		lar = LockAcquire(&tag, ShareLock, false, !wait);
+		if (lar == LOCKACQUIRE_NOT_AVAIL)
+		{
+			result = false;
+			break;
+		}
+
+		LockRelease(&tag, ShareLock, false);
+
+		if (xidlist.next == NULL)
+		{
+			result = true;
+			break;
+		}
+		xidlist = *xidlist.next;
+	}
+
+	/* Free all pallocs made by TwoPhaseGetXidByVXid() */
+	while (xidlist_ptr)
+	{
+		void *chunk = xidlist_ptr;
+		xidlist_ptr = xidlist_ptr->next;
+		pfree(chunk);
+	}
+	return result;
+}
+
 /*
  *		VirtualXactLock
  *
@@ -4459,25 +4542,18 @@ VirtualXactLockTableCleanup(void)
 bool
 VirtualXactLock(VirtualTransactionId vxid, bool wait)
 {
-	LOCKTAG		tag;
-	PGPROC	   *proc;
+	LOCKTAG			tag;
+	PGPROC		   *proc;
+	TransactionId	xid = InvalidTransactionId;
 
 	Assert(VirtualTransactionIdIsValid(vxid));
 
+	/*
+	 * Already prepared transactions don't hold vxid locks.  The
+	 * LocalTransactionId is always a normal, locked XID.
+	 */
 	if (VirtualTransactionIdIsPreparedXact(vxid))
-	{
-		LockAcquireResult lar;
-
-		/*
-		 * Prepared transactions don't hold vxid locks.  The
-		 * LocalTransactionId is always a normal, locked XID.
-		 */
-		SET_LOCKTAG_TRANSACTION(tag, vxid.localTransactionId);
-		lar = LockAcquire(&tag, ShareLock, false, !wait);
-		if (lar != LOCKACQUIRE_NOT_AVAIL)
-			LockRelease(&tag, ShareLock, false);
-		return lar != LOCKACQUIRE_NOT_AVAIL;
-	}
+		return PreparedXactLock(vxid, vxid.localTransactionId, wait);
 
 	SET_LOCKTAG_VIRTUALTRANSACTION(tag, vxid);
 
@@ -4491,7 +4567,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
 	 */
 	proc = BackendIdGetProc(vxid.backendId);
 	if (proc == NULL)
-		return true;
+		return PreparedXactLock(vxid, InvalidTransactionId, wait);
 
 	/*
 	 * We must acquire this lock before checking the backendId and lxid
@@ -4505,9 +4581,12 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
 		|| proc->fpLocalTransactionId != vxid.localTransactionId)
 	{
 		LWLockRelease(&proc->fpInfoLock);
-		return true;
+		return PreparedXactLock(vxid, InvalidTransactionId, wait);
 	}
 
+	/* Save the xid to test if transaction coverted to 2pc later */
+	xid = proc->xid;
+
 	/*
 	 * If we aren't asked to wait, there's no need to set up a lock table
 	 * entry.  The transaction is still in progress, so just return false.
@@ -4559,7 +4638,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
 	(void) LockAcquire(&tag, ShareLock, false, false);
 
 	LockRelease(&tag, ShareLock, false);
-	return true;
+	return PreparedXactLock(vxid, xid, wait);;
 }
 
 /*
diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h
index 91786da784..ba0ac17359 100644
--- a/src/include/access/twophase.h
+++ b/src/include/access/twophase.h
@@ -25,6 +25,17 @@
  */
 typedef struct GlobalTransactionData *GlobalTransaction;
 
+/* 
+ * XidListEntry is expected to be used as list very rarely. Under normal
+ * circumstances TwoPhaseGetXidByVXid() returns only one xid.
+ * But under certain conditions can return many xids or nothing.
+ */
+typedef struct XidListEntry
+{
+	TransactionId xid;
+	struct XidListEntry* next;
+} XidListEntry;
+
 /* GUC variable */
 extern PGDLLIMPORT int max_prepared_xacts;
 
@@ -58,4 +69,6 @@ extern void PrepareRedoAdd(char *buf, XLogRecPtr start_lsn,
 						   XLogRecPtr end_lsn, RepOriginId origin_id);
 extern void PrepareRedoRemove(TransactionId xid, bool giveWarning);
 extern void restoreTwoPhaseData(void);
+
+extern XidListEntry TwoPhaseGetXidByVXid(VirtualTransactionId vxid);
 #endif							/* TWOPHASE_H */
-- 
2.24.3 (Apple Git-128)