From 19cf52e6cc33b9e126775f28269ccccb6ddf7e30 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Thu, 13 Dec 2018 06:12:25 +0300 Subject: [PATCH] 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 afd8494b42..091268175e 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)) -- 2.40.0