postgres.git / summary / log / commit / refs

commit    19cf52e6cc33b9e126775f28269ccccb6ddf7e30
Author:   Alexander Korotkov <akorotkov@postgresql.org>
Date:     Thu Dec 13 03:12:25 2018 +0000
First released in: 12.0, 9.4.21, 9.5.16, 9.6.12, 10.7, 11.2

    Prevent deadlock in ginRedoDeletePage()
    
    On standby ginRedoDeletePage() can work concurrently with read-only queries.
    Those queries can traverse posting tree in two ways.
    1) Using rightlinks by ginStepRight(), which locks the next page before
       unlocking its left sibling.
    2) Using downlinks by ginFindLeafPage(), which locks at most one page at time.
    
    Original lock order was: page, parent, left sibling.  That lock order can
    deadlock with ginStepRight().  In order to prevent deadlock this commit changes
    lock order to: left sibling, page, parent.  Note, that position of parent in
    locking order seems insignificant, because we only lock one page at time while
    traversing downlinks.
    
    Reported-by: Chen Huajun
    Diagnosed-by: Chen Huajun, Peter Geoghegan, Andrey Borodin
    Discussion: https://postgr.es/m/31a702a.14dd.166c1366ac1.Coremail.chjischj%40163.com
    Author: Alexander Korotkov
    Backpatch-through: 9.4


 src/backend/access/gin/ginxlog.c | 44 +++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
index afd8494b428..091268175e5 100644
--- a/src/backend/access/gin/ginxlog.c
+++ b/src/backend/access/gin/ginxlog.c
@@ -693,6 +693,29 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 	Buffer		lbuffer;
 	Page		page;
 
+	/*
+	 * Lock left page first in order to prevent possible deadlock with
+	 * ginStepRight().
+	 */
+	if (record->xl_info & XLR_BKP_BLOCK(2))
+		(void) RestoreBackupBlock(lsn, record, 2, false, false);
+	else if (data->leftBlkno != InvalidBlockNumber)
+	{
+		lbuffer = XLogReadBuffer(data->node, data->leftBlkno, false);
+		if (BufferIsValid(lbuffer))
+		{
+			page = BufferGetPage(lbuffer);
+			if (lsn > PageGetLSN(page))
+			{
+				Assert(GinPageIsData(page));
+				GinPageGetOpaque(page)->rightlink = data->rightLink;
+				PageSetLSN(page, lsn);
+				MarkBufferDirty(lbuffer);
+			}
+			UnlockReleaseBuffer(lbuffer);
+		}
+	}
+
 	if (record->xl_info & XLR_BKP_BLOCK(0))
 		dbuffer = RestoreBackupBlock(lsn, record, 0, false, true);
 	else
@@ -730,25 +753,8 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 		}
 	}
 
-	if (record->xl_info & XLR_BKP_BLOCK(2))
-		(void) RestoreBackupBlock(lsn, record, 2, false, false);
-	else if (data->leftBlkno != InvalidBlockNumber)
-	{
-		lbuffer = XLogReadBuffer(data->node, data->leftBlkno, false);
-		if (BufferIsValid(lbuffer))
-		{
-			page = BufferGetPage(lbuffer);
-			if (lsn > PageGetLSN(page))
-			{
-				Assert(GinPageIsData(page));
-				GinPageGetOpaque(page)->rightlink = data->rightLink;
-				PageSetLSN(page, lsn);
-				MarkBufferDirty(lbuffer);
-			}
-			UnlockReleaseBuffer(lbuffer);
-		}
-	}
-
+	if (BufferIsValid(lbuffer))
+		UnlockReleaseBuffer(lbuffer);
 	if (BufferIsValid(pbuffer))
 		UnlockReleaseBuffer(pbuffer);
 	if (BufferIsValid(dbuffer))


[parent: a96722938ed7]