v16-0005-Avoid-extra-buffer-lookup-when-prefetching-WAL-b.patch

text/x-patch

Filename: v16-0005-Avoid-extra-buffer-lookup-when-prefetching-WAL-b.patch
Type: text/x-patch
Part: 4
Message: Re: WIP: WAL prefetch (another approach)

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v16-0005
Subject: Avoid extra buffer lookup when prefetching WAL blocks.
File+
src/backend/access/transam/xlog.c 1 1
src/backend/access/transam/xlogprefetch.c 3 3
src/backend/access/transam/xlogreader.c 13 0
src/backend/access/transam/xlogutils.c 19 4
src/backend/storage/buffer/bufmgr.c 1 0
src/backend/storage/freespace/freespace.c 2 1
src/include/access/xlogreader.h 7 0
src/include/access/xlogutils.h 2 1
From 4af3b9b7c0aa31c180ed2ab81498a833d9fcd4f1 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Thu, 18 Mar 2021 13:01:25 +1300
Subject: [PATCH v16 5/5] Avoid extra buffer lookup when prefetching WAL
 blocks.

Provide a some workspace in decoded WAL records to remember which buffer
recently contained we found a block cached in, so that we can try to use
ReadRecentBuffer() while replaying.

Discussion: https://postgr.es/m/CA%2BhUKGJ4VJN8ttxScUFM8dOKX0BrBiboo5uz1cq%3DAovOddfHpA%40mail.gmail.com
---
 src/backend/access/transam/xlog.c         |  2 +-
 src/backend/access/transam/xlogprefetch.c |  6 +++---
 src/backend/access/transam/xlogreader.c   | 13 +++++++++++++
 src/backend/access/transam/xlogutils.c    | 23 +++++++++++++++++++----
 src/backend/storage/buffer/bufmgr.c       |  1 +
 src/backend/storage/freespace/freespace.c |  3 ++-
 src/include/access/xlogreader.h           |  7 +++++++
 src/include/access/xlogutils.h            |  3 ++-
 8 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e24e5f0c3c..8798185cae 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1463,7 +1463,7 @@ checkXLogConsistency(XLogReaderState *record)
 		 * temporary page.
 		 */
 		buf = XLogReadBufferExtended(rnode, forknum, blkno,
-									 RBM_NORMAL_NO_LOG);
+									 RBM_NORMAL_NO_LOG, InvalidBuffer);
 		if (!BufferIsValid(buf))
 			continue;
 
diff --git a/src/backend/access/transam/xlogprefetch.c b/src/backend/access/transam/xlogprefetch.c
index 7224d882cb..f14636de3e 100644
--- a/src/backend/access/transam/xlogprefetch.c
+++ b/src/backend/access/transam/xlogprefetch.c
@@ -636,10 +636,10 @@ XLogPrefetcherScanBlocks(XLogPrefetcher *prefetcher)
 		if (BufferIsValid(prefetch.recent_buffer))
 		{
 			/*
-			 * It was already cached, so do nothing.  Perhaps in future we
-			 * could remember the buffer so that recovery doesn't have to look
-			 * it up again.
+			 * It was already cached, so do nothing.  We'll remember the
+			 * buffer, so that recovery can try to avoid looking it up again.
 			 */
+			block->recent_buffer = prefetch.recent_buffer;
 			XLogPrefetchIncrement(&Stats->skip_hit);
 		}
 		else if (prefetch.initiated_io)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index a85b718d54..4f0ae8b71c 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1646,6 +1646,8 @@ DecodeXLogRecord(XLogReaderState *state,
 			blk->has_image = ((fork_flags & BKPBLOCK_HAS_IMAGE) != 0);
 			blk->has_data = ((fork_flags & BKPBLOCK_HAS_DATA) != 0);
 
+			blk->recent_buffer = InvalidBuffer;
+
 			COPY_HEADER_FIELD(&blk->data_len, sizeof(uint16));
 			/* cross-check that the HAS_DATA flag is set iff data_length > 0 */
 			if (blk->has_data && blk->data_len == 0)
@@ -1853,6 +1855,15 @@ err:
 bool
 XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
 				   RelFileNode *rnode, ForkNumber *forknum, BlockNumber *blknum)
+{
+	return XLogRecGetRecentBuffer(record, block_id, rnode, forknum, blknum,
+								  NULL);
+}
+
+bool
+XLogRecGetRecentBuffer(XLogReaderState *record, uint8 block_id,
+					   RelFileNode *rnode, ForkNumber *forknum,
+					   BlockNumber *blknum, Buffer *recent_buffer)
 {
 	DecodedBkpBlock *bkpb;
 
@@ -1867,6 +1878,8 @@ XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
 		*forknum = bkpb->forknum;
 	if (blknum)
 		*blknum = bkpb->blkno;
+	if (recent_buffer)
+		*recent_buffer = bkpb->recent_buffer;
 	return true;
 }
 
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 5cd1c8ab1b..41f56ff856 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -335,11 +335,13 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
 	RelFileNode rnode;
 	ForkNumber	forknum;
 	BlockNumber blkno;
+	Buffer		recent_buffer;
 	Page		page;
 	bool		zeromode;
 	bool		willinit;
 
-	if (!XLogRecGetBlockTag(record, block_id, &rnode, &forknum, &blkno))
+	if (!XLogRecGetRecentBuffer(record, block_id, &rnode, &forknum, &blkno,
+								&recent_buffer))
 	{
 		/* Caller specified a bogus block_id */
 		elog(PANIC, "failed to locate backup block with ID %d", block_id);
@@ -361,7 +363,8 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
 	{
 		Assert(XLogRecHasBlockImage(record, block_id));
 		*buf = XLogReadBufferExtended(rnode, forknum, blkno,
-									  get_cleanup_lock ? RBM_ZERO_AND_CLEANUP_LOCK : RBM_ZERO_AND_LOCK);
+									  get_cleanup_lock ? RBM_ZERO_AND_CLEANUP_LOCK : RBM_ZERO_AND_LOCK,
+									  recent_buffer);
 		page = BufferGetPage(*buf);
 		if (!RestoreBlockImage(record, block_id, page))
 			elog(ERROR, "failed to restore block image");
@@ -390,7 +393,8 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
 	}
 	else
 	{
-		*buf = XLogReadBufferExtended(rnode, forknum, blkno, mode);
+		*buf = XLogReadBufferExtended(rnode, forknum, blkno, mode,
+									  recent_buffer);
 		if (BufferIsValid(*buf))
 		{
 			if (mode != RBM_ZERO_AND_LOCK && mode != RBM_ZERO_AND_CLEANUP_LOCK)
@@ -437,7 +441,8 @@ XLogReadBufferForRedoExtended(XLogReaderState *record,
  */
 Buffer
 XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
-					   BlockNumber blkno, ReadBufferMode mode)
+					   BlockNumber blkno, ReadBufferMode mode,
+					   Buffer recent_buffer)
 {
 	BlockNumber lastblock;
 	Buffer		buffer;
@@ -445,6 +450,15 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 
 	Assert(blkno != P_NEW);
 
+	/* Do we have a clue where the buffer might be already? */
+	if (BufferIsValid(recent_buffer) &&
+		mode == RBM_NORMAL &&
+		ReadRecentBuffer(rnode, forknum, blkno, recent_buffer))
+	{
+		buffer = recent_buffer;
+		goto recent_buffer_fast_path;
+	}
+
 	/* Open the relation at smgr level */
 	smgr = smgropen(rnode, InvalidBackendId);
 
@@ -503,6 +517,7 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 		}
 	}
 
+recent_buffer_fast_path:
 	if (mode == RBM_NORMAL)
 	{
 		/* check that page has been initialized */
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 0e5f92d92b..0cc1eb7971 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -651,6 +651,7 @@ ReadRecentBuffer(RelFileNode rnode, ForkNumber forkNum, BlockNumber blockNum,
 	if (!BufferIsLocal(recent_buffer))
 		UnpinBuffer(bufHdr, true);
 
+
 	return false;
 }
 
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index 8c12dda238..cfa0414e5a 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -210,7 +210,8 @@ XLogRecordPageWithFreeSpace(RelFileNode rnode, BlockNumber heapBlk,
 	blkno = fsm_logical_to_physical(addr);
 
 	/* If the page doesn't exist already, extend */
-	buf = XLogReadBufferExtended(rnode, FSM_FORKNUM, blkno, RBM_ZERO_ON_ERROR);
+	buf = XLogReadBufferExtended(rnode, FSM_FORKNUM, blkno, RBM_ZERO_ON_ERROR,
+								 InvalidBuffer);
 	LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
 
 	page = BufferGetPage(buf);
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e213c68256..be370e9015 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -39,6 +39,7 @@
 #endif
 
 #include "access/xlogrecord.h"
+#include "storage/buf.h"
 
 /* WALOpenSegment represents a WAL segment being read. */
 typedef struct WALOpenSegment
@@ -126,6 +127,9 @@ typedef struct
 	ForkNumber	forknum;
 	BlockNumber blkno;
 
+	/* Workspace for remembering last known buffer holding this block. */
+	Buffer		recent_buffer;
+
 	/* copy of the fork_flags field from the XLogRecordBlockHeader */
 	uint8		flags;
 
@@ -377,5 +381,8 @@ extern char *XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *
 extern bool XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
 							   RelFileNode *rnode, ForkNumber *forknum,
 							   BlockNumber *blknum);
+extern bool XLogRecGetRecentBuffer(XLogReaderState *record, uint8 block_id,
+								   RelFileNode *rnode, ForkNumber *forknum,
+								   BlockNumber *blknum, Buffer *recent_buffer);
 
 #endif							/* XLOGREADER_H */
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 73d3f4a129..0ee4a7c52c 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -42,7 +42,8 @@ extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
 													Buffer *buf);
 
 extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
-									 BlockNumber blkno, ReadBufferMode mode);
+									 BlockNumber blkno, ReadBufferMode mode,
+									 Buffer recent_buffer);
 
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
-- 
2.30.1