Fix wrong backpatching of ginRedoDeletePage() deadlock fix

akorotkov · akorotkov · commit bf0e5a73be20 · 2018-12-13T22:32:05.000+03:00
19cf52e changes lock order in ginRedoDeletePage(). But did it in a wrong way due to oversight during backpatching. This commit fixes that. Reported-by: Bruce Momjian Discussion: https://postgr.es/m/20181213153232.GA10664%40momjian.us
diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
@@ -690,15 +690,15 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 	ginxlogDeletePage *data = (ginxlogDeletePage *) XLogRecGetData(record);
 	Buffer		dbuffer;
 	Buffer		pbuffer;
-	Buffer		lbuffer;
+	Buffer		lbuffer = InvalidBlockNumber;
 	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);
+		lbuffer = RestoreBackupBlock(lsn, record, 2, false, true);
 	else if (data->leftBlkno != InvalidBlockNumber)
 	{
 		lbuffer = XLogReadBuffer(data->node, data->leftBlkno, false);
@@ -712,7 +712,6 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 				PageSetLSN(page, lsn);
 				MarkBufferDirty(lbuffer);
 			}
-			UnlockReleaseBuffer(lbuffer);
 		}
 	}
 
@@ -735,7 +734,7 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 	}
 
 	if (record->xl_info & XLR_BKP_BLOCK(1))
-		pbuffer = RestoreBackupBlock(lsn, record, 1, false, true);
+		(void) RestoreBackupBlock(lsn, record, 1, false, false);
 	else
 	{
 		pbuffer = XLogReadBuffer(data->node, data->parentBlkno, false);
@@ -750,13 +749,12 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
 				PageSetLSN(page, lsn);
 				MarkBufferDirty(pbuffer);
 			}
+			UnlockReleaseBuffer(pbuffer);
 		}
 	}
 
 	if (BufferIsValid(lbuffer))
 		UnlockReleaseBuffer(lbuffer);
-	if (BufferIsValid(pbuffer))
-		UnlockReleaseBuffer(pbuffer);
 	if (BufferIsValid(dbuffer))
 		UnlockReleaseBuffer(dbuffer);
 }

Original file line number	Diff line number	Diff line change
`@@ -690,15 +690,15 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)`
`690`	`690`	`ginxlogDeletePage data = (ginxlogDeletePage ) XLogRecGetData(record);`
`691`	`691`	`Buffer dbuffer;`
`692`	`692`	`Buffer pbuffer;`
`693`		`- Buffer lbuffer;`
	`693`	`+ Buffer lbuffer = InvalidBlockNumber;`
`694`	`694`	`Page page;`
`695`	`695`
`696`	`696`	`/*`
`697`	`697`	`* Lock left page first in order to prevent possible deadlock with`
`698`	`698`	`* ginStepRight().`
`699`	`699`	`*/`
`700`	`700`	`if (record->xl_info & XLR_BKP_BLOCK(2))`
`701`		`- (void) RestoreBackupBlock(lsn, record, 2, false, false);`
	`701`	`+ lbuffer = RestoreBackupBlock(lsn, record, 2, false, true);`
`702`	`702`	`else if (data->leftBlkno != InvalidBlockNumber)`
`703`	`703`	`{`
`704`	`704`	`lbuffer = XLogReadBuffer(data->node, data->leftBlkno, false);`
`@@ -712,7 +712,6 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)`
`712`	`712`	`PageSetLSN(page, lsn);`
`713`	`713`	`MarkBufferDirty(lbuffer);`
`714`	`714`	`}`
`715`		`- UnlockReleaseBuffer(lbuffer);`
`716`	`715`	`}`
`717`	`716`	`}`
`718`	`717`
`@@ -735,7 +734,7 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)`
`735`	`734`	`}`
`736`	`735`
`737`	`736`	`if (record->xl_info & XLR_BKP_BLOCK(1))`
`738`		`- pbuffer = RestoreBackupBlock(lsn, record, 1, false, true);`
	`737`	`+ (void) RestoreBackupBlock(lsn, record, 1, false, false);`
`739`	`738`	`else`
`740`	`739`	`{`
`741`	`740`	`pbuffer = XLogReadBuffer(data->node, data->parentBlkno, false);`
`@@ -750,13 +749,12 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)`
`750`	`749`	`PageSetLSN(page, lsn);`
`751`	`750`	`MarkBufferDirty(pbuffer);`
`752`	`751`	`}`
	`752`	`+ UnlockReleaseBuffer(pbuffer);`
`753`	`753`	`}`
`754`	`754`	`}`
`755`	`755`
`756`	`756`	`if (BufferIsValid(lbuffer))`
`757`	`757`	`UnlockReleaseBuffer(lbuffer);`
`758`		`- if (BufferIsValid(pbuffer))`
`759`		`- UnlockReleaseBuffer(pbuffer);`
`760`	`758`	`if (BufferIsValid(dbuffer))`
`761`	`759`	`UnlockReleaseBuffer(dbuffer);`
`762`	`760`	`}`