v2-0001-Fix-cleanup-lock-acquisition-in-SPLIT_ALLOCATE_PA.patch

application/octet-stream

Filename: v2-0001-Fix-cleanup-lock-acquisition-in-SPLIT_ALLOCATE_PA.patch
Type: application/octet-stream
Part: 1
Message: Re: hash_xlog_split_allocate_page: failed to acquire cleanup lock

Patch

Format: format-patch
Series: patch v2-0001
Subject: Fix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.
File+
src/backend/access/hash/hashpage.c 2 1
src/backend/access/hash/hash_xlog.c 2 3
From 4a242d752fa5f6103b47363291f9094255524be6 Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Thu, 6 Oct 2022 11:03:31 +0530
Subject: [PATCH v2 1/2] Fix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE
 replay.

During XLOG_HASH_SPLIT_ALLOCATE_PAGE replay, we were checking for a
cleanup lock on the new bucket page after acquiring an exclusive lock on
it and raising a PANIC error on failure. However, it is quite possible
that checkpointer can acquire the pin on the same page before acquiring a
lock on it and then the replay will lead to an error.
---
 src/backend/access/hash/hash_xlog.c | 5 ++---
 src/backend/access/hash/hashpage.c  | 3 ++-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index e88213c..a24a1c3 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -351,11 +351,10 @@ hash_xlog_split_allocate_page(XLogReaderState *record)
 	}
 
 	/* replay the record for new bucket */
-	newbuf = XLogInitBufferForRedo(record, 1);
+	XLogReadBufferForRedoExtended(record, 1, RBM_ZERO_AND_CLEANUP_LOCK, true,
+								  &newbuf);
 	_hash_initbuf(newbuf, xlrec->new_bucket, xlrec->new_bucket,
 				  xlrec->new_bucket_flag, true);
-	if (!IsBufferCleanupOK(newbuf))
-		elog(PANIC, "hash_xlog_split_allocate_page: failed to acquire cleanup lock");
 	MarkBufferDirty(newbuf);
 	PageSetLSN(BufferGetPage(newbuf), lsn);
 
diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index d2edcd4..05fcf76 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -807,7 +807,8 @@ restart_expand:
 	 * before changing the metapage's mapping info, in case we can't get the
 	 * disk space.  Ideally, we don't need to check for cleanup lock on new
 	 * bucket as no other backend could find this bucket unless meta page is
-	 * updated.  However, it is good to be consistent with old bucket locking.
+	 * updated and we initialize the page just before it.  However, it is just
+	 * to be consistent with old bucket locking.
 	 */
 	buf_nblkno = _hash_getnewbuf(rel, start_nblkno, MAIN_FORKNUM);
 	if (!IsBufferCleanupOK(buf_nblkno))
-- 
1.8.3.1