From 358cde320b17535011b0a912e5c9ce1453666ed1 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 2 Aug 2015 20:08:10 +0300 Subject: [PATCH] Fix race condition that lead to WALInsertLock deadlock with commit_delay. If a call to WaitForXLogInsertionsToFinish() returned a value in the middle of a page, and another backend then started to insert a record to the same page, and then you called WaitXLogInsertionsToFinish() again, the second call might return a smaller value than the first call. The problem was in GetXLogBuffer(), which always updated the insertingAt value to the beginning of the requested page, not the actual requested location. Because of that, the second call might return a xlog pointer to the beginning of the page, while the first one returned a later position on the same page. XLogFlush() performs two calls to WaitXLogInsertionsToFinish() in succession, and holds WALWriteLock on the second call, which can deadlock if the second call to WaitXLogInsertionsToFinish() blocks. Reported by Spiros Ioannou. Backpatch to 9.4, where the more scalable WALInsertLock mechanism, and this bug, was introduced. --- src/backend/access/transam/xlog.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 939813e7b71..f06b51dde37 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1664,11 +1664,32 @@ GetXLogBuffer(XLogRecPtr ptr) endptr = XLogCtl->xlblocks[idx]; if (expectedEndPtr != endptr) { + XLogRecPtr initializedUpto; + /* - * Let others know that we're finished inserting the record up to the - * page boundary. + * Before calling AdvanceXLInsertBuffer(), which can block, let others + * know how far we're finished with inserting the record. + * + * NB: If 'ptr' points to just after the page header, advertise a + * position at the beginning of the page rather than 'ptr' itself. If + * there are no other insertions running, someone might try to flush + * up to our advertised location. If we advertised a position after + * the page header, someone might try to flush the page header, even + * though page might actually not be initialized yet. As the first + * inserter on the page, we are effectively responsible for making + * sure that it's initialized, before we let insertingAt to move past + * the page header. */ - WALInsertLockUpdateInsertingAt(expectedEndPtr - XLOG_BLCKSZ); + if (ptr % XLOG_BLCKSZ == SizeOfXLogShortPHD && + ptr % XLOG_SEG_SIZE > XLOG_BLCKSZ) + initializedUpto = ptr - SizeOfXLogShortPHD; + else if (ptr % XLOG_BLCKSZ == SizeOfXLogLongPHD && + ptr % XLOG_SEG_SIZE < XLOG_BLCKSZ) + initializedUpto = ptr - SizeOfXLogLongPHD; + else + initializedUpto = ptr; + + WALInsertLockUpdateInsertingAt(initializedUpto); AdvanceXLInsertBuffer(ptr, false); endptr = XLogCtl->xlblocks[idx];