postgres.git / summary / log / commit / refs

commit    fe0cc5ba5372eca5e9eec1b317dffa0adf7b1049
Author:   Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:     Wed Jan 03 20:26:20 2018 +0000
First released in: 11.0, 9.4.16, 9.5.11, 9.6.7, 10.2

    Make XactLockTableWait work for transactions that are not yet self-locked
    
    XactLockTableWait assumed that its xid argument has already added itself
    to the lock table.  That assumption led to another assumption that if
    locking the xid has succeeded but the xid is reported as still in
    progress, then the input xid must have been a subtransaction.
    
    These assumptions hold true for the original uses of this code in
    locking related to on-disk tuples, but they break down in logical
    replication slot snapshot building -- in particular, when a standby
    snapshot logged contains an xid that's already in ProcArray but not yet
    in the lock table.  This leads to assertion failures that can be
    reproduced all the way back to 9.4, when logical decoding was
    introduced.
    
    To fix, change SubTransGetParent to SubTransGetTopmostTransaction which
    has a slightly different API: it returns the argument Xid if there is no
    parent, and it goes all the way to the top instead of moving up the
    levels one by one.  Also, to avoid busy-waiting, add a 1ms sleep to give
    the other process time to register itself in the lock table.
    
    For consistency, change ConditionalXactLockTableWait the same way.
    
    Author: Petr Jelínek
    Discussion: https://postgr.es/m/1B3E32D8-FCF4-40B4-AEF9-5C0E3AC57969@postgrespro.ru
    Reported-by: Konstantin Knizhnik
    Diagnosed-by: Stas Kelvich, Petr Jelínek
    Reviewed-by: Andres Freund, Robert Haas


 src/backend/storage/lmgr/lmgr.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 7b08555b071..9a2f94beaf7 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -557,6 +557,7 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 	LOCKTAG		tag;
 	XactLockTableWaitInfo info;
 	ErrorContextCallback callback;
+	bool		first = true;
 
 	/*
 	 * If an operation is specified, set up our verbose error context
@@ -590,7 +591,26 @@ XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
 
 		if (!TransactionIdIsInProgress(xid))
 			break;
-		xid = SubTransGetParent(xid);
+
+		/*
+		 * If the Xid belonged to a subtransaction, then the lock would have
+		 * gone away as soon as it was finished; for correct tuple visibility,
+		 * the right action is to wait on its parent transaction to go away.
+		 * But instead of going levels up one by one, we can just wait for the
+		 * topmost transaction to finish with the same end result, which also
+		 * incurs less locktable traffic.
+		 *
+		 * Some uses of this function don't involve tuple visibility -- such
+		 * as when building snapshots for logical decoding.  It is possible to
+		 * see a transaction in ProcArray before it registers itself in the
+		 * locktable.  The topmost transaction in that case is the same xid,
+		 * so we try again after a short sleep.  (Don't sleep the first time
+		 * through, to avoid slowing down the normal case.)
+		 */
+		if (!first)
+			pg_usleep(1000L);
+		first = false;
+		xid = SubTransGetTopmostTransaction(xid);
 	}
 
 	if (oper != XLTW_None)
@@ -607,6 +627,7 @@ bool
 ConditionalXactLockTableWait(TransactionId xid)
 {
 	LOCKTAG		tag;
+	bool		first = true;
 
 	for (;;)
 	{
@@ -622,7 +643,12 @@ ConditionalXactLockTableWait(TransactionId xid)
 
 		if (!TransactionIdIsInProgress(xid))
 			break;
-		xid = SubTransGetParent(xid);
+
+		/* See XactLockTableWait about this case */
+		if (!first)
+			pg_usleep(1000L);
+		first = false;
+		xid = SubTransGetTopmostTransaction(xid);
 	}
 
 	return true;


[parent: 0f38a1d85abe]