Using read_stream in index vacuum

From c21498c4c18432a6c13ef1989182e0f84a171093 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v2 1/2] Prototype B-tree vacuum streamlineing

---
 src/backend/access/nbtree/nbtree.c | 66 ++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index f4f79f2706..b6762e9c30 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -88,7 +88,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static void btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -1040,6 +1040,45 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	needLock = !RELATION_IS_LOCAL(rel);
 
 	scanblkno = BTREE_METAPAGE + 1;
+
+	{
+		/* streamline reading most of index data */
+		BlockRangeReadStreamPrivate p;
+		ReadStream *stream = NULL;
+		p.current_blocknum = scanblkno;
+		if (needLock)
+			LockRelationForExtension(rel, ExclusiveLock);
+		/* We only streamline number of blocks that are know at the beginning */
+		p.last_exclusive = RelationGetNumberOfBlocks(rel);
+		if (needLock)
+			UnlockRelationForExtension(rel, ExclusiveLock);
+		stream = read_stream_begin_relation(READ_STREAM_FULL,
+											info->strategy,
+											rel,
+											MAIN_FORKNUM,
+											block_range_read_stream_cb,
+											&p,
+											0);
+		for (; scanblkno < p.last_exclusive; scanblkno++)
+		{
+			Buffer buf = read_stream_next_buffer(stream, NULL);
+			/*
+			 * We expect that blocks are returned in order.
+			 * However, we do not depent on it much, and in future ths
+			 * expetation might change.
+			 * Currently, this expectation only matters for progress reporting.
+			 */
+			Assert(BufferGetBlockNumber(buf) == scanblkno);
+			btvacuumpage(&vstate, buf);
+			if (info->report_progress)
+				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
+											 scanblkno);
+		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		read_stream_end(stream);
+	}
+
+	/* Now we have to process pages created after streamlined vacuum */
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -1059,7 +1098,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		/* Iterate over pages, then loop back to recheck length */
 		for (; scanblkno < num_pages; scanblkno++)
 		{
-			btvacuumpage(&vstate, scanblkno);
+			Buffer buf = ReadBufferExtended(rel, MAIN_FORKNUM, scanblkno, RBM_NORMAL,
+							 info->strategy);
+			btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
 											 scanblkno);
@@ -1096,7 +1137,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * recycled (i.e. before the page split).
  */
 static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1107,7 +1148,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1121,14 +1162,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1415,6 +1448,15 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
 }
-- 
2.39.5

From cf650befbbe48a14f212fb847a12aee16c2e826f Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Sun, 20 Oct 2024 10:09:59 +0000
Subject: [PATCH v2 2/2] refactor

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/access/nbtree/nbtree.c | 56 +++++++++---------------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index b6762e9c30..1d493aa680 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -971,6 +971,8 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	BlockNumber num_pages;
 	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1041,44 +1043,6 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 
 	scanblkno = BTREE_METAPAGE + 1;
 
-	{
-		/* streamline reading most of index data */
-		BlockRangeReadStreamPrivate p;
-		ReadStream *stream = NULL;
-		p.current_blocknum = scanblkno;
-		if (needLock)
-			LockRelationForExtension(rel, ExclusiveLock);
-		/* We only streamline number of blocks that are know at the beginning */
-		p.last_exclusive = RelationGetNumberOfBlocks(rel);
-		if (needLock)
-			UnlockRelationForExtension(rel, ExclusiveLock);
-		stream = read_stream_begin_relation(READ_STREAM_FULL,
-											info->strategy,
-											rel,
-											MAIN_FORKNUM,
-											block_range_read_stream_cb,
-											&p,
-											0);
-		for (; scanblkno < p.last_exclusive; scanblkno++)
-		{
-			Buffer buf = read_stream_next_buffer(stream, NULL);
-			/*
-			 * We expect that blocks are returned in order.
-			 * However, we do not depent on it much, and in future ths
-			 * expetation might change.
-			 * Currently, this expectation only matters for progress reporting.
-			 */
-			Assert(BufferGetBlockNumber(buf) == scanblkno);
-			btvacuumpage(&vstate, buf);
-			if (info->report_progress)
-				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
-		}
-		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
-		read_stream_end(stream);
-	}
-
-	/* Now we have to process pages created after streamlined vacuum */
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -1095,16 +1059,28 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		/* Quit if we've scanned the whole relation */
 		if (scanblkno >= num_pages)
 			break;
+
+		p.current_blocknum = scanblkno;
+		p.last_exclusive = num_pages;
+		stream = read_stream_begin_relation(READ_STREAM_FULL,
+											info->strategy,
+											rel,
+											MAIN_FORKNUM,
+											block_range_read_stream_cb,
+											&p,
+											0);
+
 		/* Iterate over pages, then loop back to recheck length */
 		for (; scanblkno < num_pages; scanblkno++)
 		{
-			Buffer buf = ReadBufferExtended(rel, MAIN_FORKNUM, scanblkno, RBM_NORMAL,
-							 info->strategy);
+			Buffer buf = read_stream_next_buffer(stream, NULL);
 			btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
 											 scanblkno);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		read_stream_end(stream);
 	}
 
 	/* Set statistics num_pages field to final size of index */
-- 
2.39.5

From 185c9469feff2deca7801882a3747008aab22966 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v3] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/access/nbtree/nbtree.c | 46 ++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index f4f79f2..2dc89cb 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -88,7 +88,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static void btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -971,6 +971,8 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	BlockNumber num_pages;
 	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1040,6 +1042,13 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	needLock = !RELATION_IS_LOCAL(rel);
 
 	scanblkno = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -1056,15 +1065,27 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		/* Quit if we've scanned the whole relation */
 		if (scanblkno >= num_pages)
 			break;
+
+		p.current_blocknum = scanblkno;
+		p.last_exclusive = num_pages;
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
+
 		/* Iterate over pages, then loop back to recheck length */
 		for (; scanblkno < num_pages; scanblkno++)
 		{
-			btvacuumpage(&vstate, scanblkno);
+			Buffer buf = read_stream_next_buffer(stream, NULL);
+			btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
 											 scanblkno);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1096,7 +1117,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * recycled (i.e. before the page split).
  */
 static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1107,7 +1128,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1121,14 +1142,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1415,6 +1428,15 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
 }
-- 
2.39.5 (Apple Git-154)

From 185c9469feff2deca7801882a3747008aab22966 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v4 1/4] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/access/nbtree/nbtree.c | 46 ++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index f4f79f2..2dc89cb 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -88,7 +88,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static void btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -971,6 +971,8 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	BlockNumber num_pages;
 	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1040,6 +1042,13 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	needLock = !RELATION_IS_LOCAL(rel);
 
 	scanblkno = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -1056,15 +1065,27 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		/* Quit if we've scanned the whole relation */
 		if (scanblkno >= num_pages)
 			break;
+
+		p.current_blocknum = scanblkno;
+		p.last_exclusive = num_pages;
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
+
 		/* Iterate over pages, then loop back to recheck length */
 		for (; scanblkno < num_pages; scanblkno++)
 		{
-			btvacuumpage(&vstate, scanblkno);
+			Buffer buf = read_stream_next_buffer(stream, NULL);
+			btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
 											 scanblkno);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1096,7 +1117,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * recycled (i.e. before the page split).
  */
 static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1107,7 +1128,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1121,14 +1142,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1415,6 +1428,15 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
 }
-- 
2.39.5 (Apple Git-154)

From 17d14e6ccf9b74dffdb4bdf36c7bcf933418c625 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Wed, 23 Oct 2024 20:54:02 +0300
Subject: [PATCH v4 4/4] Move reset

---
 src/backend/access/nbtree/nbtree.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 3a775e8..c9b8107 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -1073,11 +1073,6 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 
 		p.current_blocknum = scanblkno;
 		p.last_exclusive = num_pages;
-		/*
-		 * After reaching the end we have to reset stream to use it again.
-		 * Extra restart in case of just one iteration does not cost us much.
-		 */
-		read_stream_reset(stream);
 
 		/* Iterate over pages, then loop back to recheck length */
 		for (; scanblkno < num_pages; scanblkno++)
@@ -1090,6 +1085,11 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
 											 scanblkno);
 		}
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
 	}
 	read_stream_end(stream);
-- 
2.39.5 (Apple Git-154)

From e180131f802cb5ba61d8dbc04baf0b5f65f8b149 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Wed, 23 Oct 2024 20:50:33 +0300
Subject: [PATCH v4 3/4] Add asserts to address review feedback

---
 src/backend/access/nbtree/nbtree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index bd88b33..3a775e8 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -1083,6 +1083,8 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		for (; scanblkno < num_pages; scanblkno++)
 		{
 			Buffer buf = read_stream_next_buffer(stream, NULL);
+			Assert(BufferIsValid(buf));
+			Assert(BufferGetBlockNumber == scanblkno);
 			btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-- 
2.39.5 (Apple Git-154)

From e1a050a893b6f4ee2edf91367ea76b6c2aa16c4a Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Wed, 23 Oct 2024 20:46:50 +0300
Subject: [PATCH v4 2/4] Add TAP tests that verifies B-tree vacuum

---
 src/backend/access/nbtree/nbtree.c            |  5 ++
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/007_vacuum_btree.pl   | 81 +++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/007_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 2dc89cb..bd88b33 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -34,6 +34,7 @@
 #include "storage/smgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -1066,6 +1067,10 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		if (scanblkno >= num_pages)
 			break;
 
+		/* In 007_vacuum_btree test we need to coordinate two points here */
+		INJECTION_POINT("nbtree-vacuum-1");
+		INJECTION_POINT("nbtree-vacuum-2");
+
 		p.current_blocknum = scanblkno;
 		p.last_exclusive = num_pages;
 		/*
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 283ffa7..a456a5e 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -15,6 +15,7 @@ tests += {
       't/004_io_direct.pl',
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
+      't/007_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/007_vacuum_btree.pl b/src/test/modules/test_misc/t/007_vacuum_btree.pl
new file mode 100644
index 0000000..e8df4c0
--- /dev/null
+++ b/src/test/modules/test_misc/t/007_vacuum_btree.pl
@@ -0,0 +1,81 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies that B-tree vacuum can restart read stream.
+# To do so we need to insert some data during vacuum. So we wait in injection point
+# after first vacuum scan. During this wait we insert some data forcing page split.
+# this split will trigger relation extension and subsequent read_stream_reset().
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize postgres
+my $psql_err = '';
+my $psql_out = '';
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# This ensures autovacuum do not run
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# From this point, vacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-2', 'wait');");
+
+my $psql_session = $node->background_psql('postgres');
+
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		create table a as select random() r from generate_series(1,100) x;
+		create index on a(r);
+		delete from a;
+		vacuum a;
+	));
+
+#print $node->safe_psql('postgres','select * from pg_stat_activity');
+
+# Wait until an vacuum worker starts.
+$node->wait_for_event('client backend', 'nbtree-vacuum-2');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-1', 'wait');");
+
+# Here's the key point of a test: during vacuum we add some page splits.
+# This will force vacuum into doing another scan thus reseting read stream.
+$node->safe_psql('postgres',
+	"insert into a select x from generate_series(1,3000) x;");
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-2');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-2');");
+
+# Observe that second scan is reached.
+$node->wait_for_event('client backend', 'nbtree-vacuum-1');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-1');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-1');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.39.5 (Apple Git-154)

From af5aaad258125481ef9cce781feef6db9cec3cfa Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v5] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/access/nbtree/nbtree.c            | 66 ++++++++++-----
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/007_vacuum_btree.pl   | 81 +++++++++++++++++++
 3 files changed, 130 insertions(+), 18 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/007_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index f4f79f2..5e7bfbf 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -34,6 +34,7 @@
 #include "storage/smgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -88,7 +89,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static void btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -969,8 +970,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	Relation	rel = info->index;
 	BTVacState	vstate;
 	BlockNumber num_pages;
-	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1039,7 +1041,14 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	scanblkno = BTREE_METAPAGE + 1;
+	p.current_blocknum = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -1054,17 +1063,37 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 										 num_pages);
 
 		/* Quit if we've scanned the whole relation */
-		if (scanblkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; scanblkno < num_pages; scanblkno++)
+
+		/* In 007_vacuum_btree test we need to coordinate two distinguishable points here */
+		INJECTION_POINT("nbtree-vacuum-1");
+		INJECTION_POINT("nbtree-vacuum-2");
+
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(true)
 		{
-			btvacuumpage(&vstate, scanblkno);
+			BlockNumber current_block;
+			Buffer buf = read_stream_next_buffer(stream, NULL);
+			if (!BufferIsValid(buf))
+				break;
+
+			current_block = BufferGetBlockNumber(buf);
+			btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
+											 current_block);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1096,7 +1125,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * recycled (i.e. before the page split).
  */
 static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1107,7 +1136,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1121,14 +1150,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1415,6 +1436,15 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
 }
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 283ffa7..a456a5e 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -15,6 +15,7 @@ tests += {
       't/004_io_direct.pl',
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
+      't/007_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/007_vacuum_btree.pl b/src/test/modules/test_misc/t/007_vacuum_btree.pl
new file mode 100644
index 0000000..e8df4c0
--- /dev/null
+++ b/src/test/modules/test_misc/t/007_vacuum_btree.pl
@@ -0,0 +1,81 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies that B-tree vacuum can restart read stream.
+# To do so we need to insert some data during vacuum. So we wait in injection point
+# after first vacuum scan. During this wait we insert some data forcing page split.
+# this split will trigger relation extension and subsequent read_stream_reset().
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize postgres
+my $psql_err = '';
+my $psql_out = '';
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# This ensures autovacuum do not run
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# From this point, vacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-2', 'wait');");
+
+my $psql_session = $node->background_psql('postgres');
+
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		create table a as select random() r from generate_series(1,100) x;
+		create index on a(r);
+		delete from a;
+		vacuum a;
+	));
+
+#print $node->safe_psql('postgres','select * from pg_stat_activity');
+
+# Wait until an vacuum worker starts.
+$node->wait_for_event('client backend', 'nbtree-vacuum-2');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-1', 'wait');");
+
+# Here's the key point of a test: during vacuum we add some page splits.
+# This will force vacuum into doing another scan thus reseting read stream.
+$node->safe_psql('postgres',
+	"insert into a select x from generate_series(1,3000) x;");
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-2');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-2');");
+
+# Observe that second scan is reached.
+$node->wait_for_event('client backend', 'nbtree-vacuum-1');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-1');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-1');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.39.5 (Apple Git-154)

From 7719ab1ebdc3d56711505238eabe4f3f6af56af1 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v6 1/2] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/access/nbtree/nbtree.c            | 70 +++++++++++-----
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/007_vacuum_btree.pl   | 81 +++++++++++++++++++
 3 files changed, 131 insertions(+), 21 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/007_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index f4f79f2..0ee2b2b 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -34,6 +34,7 @@
 #include "storage/smgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -88,7 +89,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static BlockNumber btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -969,8 +970,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	Relation	rel = info->index;
 	BTVacState	vstate;
 	BlockNumber num_pages;
-	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1039,9 +1041,17 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	scanblkno = BTREE_METAPAGE + 1;
+	p.current_blocknum = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -1054,17 +1064,31 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 										 num_pages);
 
 		/* Quit if we've scanned the whole relation */
-		if (scanblkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; scanblkno < num_pages; scanblkno++)
+
+		/* In 007_vacuum_btree test we need to coordinate two distinguishable points here */
+		INJECTION_POINT("nbtree-vacuum-1");
+		INJECTION_POINT("nbtree-vacuum-2");
+
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			btvacuumpage(&vstate, scanblkno);
+			BlockNumber current_block = btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
+											 current_block);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1094,9 +1118,11 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * after our cycleid was acquired) whose right half page happened to reuse
  * a block that we might have processed at some point before it was
  * recycled (i.e. before the page split).
+ *
+ * Returns BlockNumber of a scanned page (not backtracked).
  */
-static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+static BlockNumber
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1107,7 +1133,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1121,14 +1147,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1164,7 +1182,7 @@ backtrack:
 					 errmsg_internal("right sibling %u of scanblkno %u unexpectedly in an inconsistent state in index \"%s\"",
 									 blkno, scanblkno, RelationGetRelationName(rel))));
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 
 		/*
@@ -1184,7 +1202,7 @@ backtrack:
 		{
 			/* Done with current scanblkno (and all lower split pages) */
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 	}
 
@@ -1415,8 +1433,18 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
+	return scanblkno;
 }
 
 /*
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 283ffa7..a456a5e 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -15,6 +15,7 @@ tests += {
       't/004_io_direct.pl',
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
+      't/007_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/007_vacuum_btree.pl b/src/test/modules/test_misc/t/007_vacuum_btree.pl
new file mode 100644
index 0000000..e8df4c0
--- /dev/null
+++ b/src/test/modules/test_misc/t/007_vacuum_btree.pl
@@ -0,0 +1,81 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies that B-tree vacuum can restart read stream.
+# To do so we need to insert some data during vacuum. So we wait in injection point
+# after first vacuum scan. During this wait we insert some data forcing page split.
+# this split will trigger relation extension and subsequent read_stream_reset().
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize postgres
+my $psql_err = '';
+my $psql_out = '';
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# This ensures autovacuum do not run
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# From this point, vacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-2', 'wait');");
+
+my $psql_session = $node->background_psql('postgres');
+
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		create table a as select random() r from generate_series(1,100) x;
+		create index on a(r);
+		delete from a;
+		vacuum a;
+	));
+
+#print $node->safe_psql('postgres','select * from pg_stat_activity');
+
+# Wait until an vacuum worker starts.
+$node->wait_for_event('client backend', 'nbtree-vacuum-2');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-1', 'wait');");
+
+# Here's the key point of a test: during vacuum we add some page splits.
+# This will force vacuum into doing another scan thus reseting read stream.
+$node->safe_psql('postgres',
+	"insert into a select x from generate_series(1,3000) x;");
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-2');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-2');");
+
+# Observe that second scan is reached.
+$node->wait_for_event('client backend', 'nbtree-vacuum-1');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-1');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-1');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.39.5 (Apple Git-154)

From 375904cfb828f5feb84dd800b581b9e006e4cc78 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 10:00:49 +0300
Subject: [PATCH v6 2/2] Use read_stream in GiST vacuum

---
 src/backend/access/gist/gistvacuum.c | 46 ++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index 24fb94f..a86122a 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -22,6 +22,7 @@
 #include "miscadmin.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/memutils.h"
 
 /* Working state needed by gistbulkdelete */
@@ -44,8 +45,7 @@ typedef struct
 
 static void gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						   IndexBulkDeleteCallback callback, void *callback_state);
-static void gistvacuumpage(GistVacState *vstate, BlockNumber blkno,
-						   BlockNumber orig_blkno);
+static void gistvacuumpage(GistVacState *vstate, Buffer buffer);
 static void gistvacuum_delete_empty_pages(IndexVacuumInfo *info,
 										  GistVacState *vstate);
 static bool gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
@@ -129,8 +129,9 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	GistVacState vstate;
 	BlockNumber num_pages;
 	bool		needLock;
-	BlockNumber blkno;
 	MemoryContext oldctx;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -208,9 +209,17 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	blkno = GIST_ROOT_BLKNO;
+	p.current_blocknum = GIST_ROOT_BLKNO;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -219,12 +228,23 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 			UnlockRelationForExtension(rel, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
-			gistvacuumpage(&vstate, blkno, blkno);
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
+		{
+			gistvacuumpage(&vstate, buf);
+		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/*
 	 * If we found any recyclable pages (and recorded them in the FSM), then
@@ -269,15 +289,16 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * are recursing to re-examine a previous page).
  */
 static void
-gistvacuumpage(GistVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
+gistvacuumpage(GistVacState *vstate, Buffer buffer)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteCallback callback = vstate->callback;
 	void	   *callback_state = vstate->callback_state;
 	Relation	rel = info->index;
-	Buffer		buffer;
+	BlockNumber orig_blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 	BlockNumber recurse_to;
+	BlockNumber blkno = orig_blkno;
 
 restart:
 	recurse_to = InvalidBlockNumber;
@@ -285,9 +306,6 @@ restart:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-								info->strategy);
-
 	/*
 	 * We are not going to stay here for a long time, aggressively grab an
 	 * exclusive lock.
@@ -450,6 +468,8 @@ restart:
 	if (recurse_to != InvalidBlockNumber)
 	{
 		blkno = recurse_to;
+		buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+									info->strategy);
 		goto restart;
 	}
 }
-- 
2.39.5 (Apple Git-154)

From 7719ab1ebdc3d56711505238eabe4f3f6af56af1 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v7 1/3] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/access/nbtree/nbtree.c            | 70 +++++++++++-----
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/007_vacuum_btree.pl   | 81 +++++++++++++++++++
 3 files changed, 131 insertions(+), 21 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/007_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index f4f79f2..0ee2b2b 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -34,6 +34,7 @@
 #include "storage/smgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -88,7 +89,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static BlockNumber btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -969,8 +970,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	Relation	rel = info->index;
 	BTVacState	vstate;
 	BlockNumber num_pages;
-	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1039,9 +1041,17 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	scanblkno = BTREE_METAPAGE + 1;
+	p.current_blocknum = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -1054,17 +1064,31 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 										 num_pages);
 
 		/* Quit if we've scanned the whole relation */
-		if (scanblkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; scanblkno < num_pages; scanblkno++)
+
+		/* In 007_vacuum_btree test we need to coordinate two distinguishable points here */
+		INJECTION_POINT("nbtree-vacuum-1");
+		INJECTION_POINT("nbtree-vacuum-2");
+
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			btvacuumpage(&vstate, scanblkno);
+			BlockNumber current_block = btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
+											 current_block);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1094,9 +1118,11 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * after our cycleid was acquired) whose right half page happened to reuse
  * a block that we might have processed at some point before it was
  * recycled (i.e. before the page split).
+ *
+ * Returns BlockNumber of a scanned page (not backtracked).
  */
-static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+static BlockNumber
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1107,7 +1133,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1121,14 +1147,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1164,7 +1182,7 @@ backtrack:
 					 errmsg_internal("right sibling %u of scanblkno %u unexpectedly in an inconsistent state in index \"%s\"",
 									 blkno, scanblkno, RelationGetRelationName(rel))));
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 
 		/*
@@ -1184,7 +1202,7 @@ backtrack:
 		{
 			/* Done with current scanblkno (and all lower split pages) */
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 	}
 
@@ -1415,8 +1433,18 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
+	return scanblkno;
 }
 
 /*
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 283ffa7..a456a5e 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -15,6 +15,7 @@ tests += {
       't/004_io_direct.pl',
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
+      't/007_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/007_vacuum_btree.pl b/src/test/modules/test_misc/t/007_vacuum_btree.pl
new file mode 100644
index 0000000..e8df4c0
--- /dev/null
+++ b/src/test/modules/test_misc/t/007_vacuum_btree.pl
@@ -0,0 +1,81 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies that B-tree vacuum can restart read stream.
+# To do so we need to insert some data during vacuum. So we wait in injection point
+# after first vacuum scan. During this wait we insert some data forcing page split.
+# this split will trigger relation extension and subsequent read_stream_reset().
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize postgres
+my $psql_err = '';
+my $psql_out = '';
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# This ensures autovacuum do not run
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# From this point, vacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-2', 'wait');");
+
+my $psql_session = $node->background_psql('postgres');
+
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		create table a as select random() r from generate_series(1,100) x;
+		create index on a(r);
+		delete from a;
+		vacuum a;
+	));
+
+#print $node->safe_psql('postgres','select * from pg_stat_activity');
+
+# Wait until an vacuum worker starts.
+$node->wait_for_event('client backend', 'nbtree-vacuum-2');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-1', 'wait');");
+
+# Here's the key point of a test: during vacuum we add some page splits.
+# This will force vacuum into doing another scan thus reseting read stream.
+$node->safe_psql('postgres',
+	"insert into a select x from generate_series(1,3000) x;");
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-2');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-2');");
+
+# Observe that second scan is reached.
+$node->wait_for_event('client backend', 'nbtree-vacuum-1');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-1');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-1');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.39.5 (Apple Git-154)

From 375904cfb828f5feb84dd800b581b9e006e4cc78 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 10:00:49 +0300
Subject: [PATCH v7 2/3] Use read_stream in GiST vacuum

---
 src/backend/access/gist/gistvacuum.c | 46 ++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index 24fb94f..a86122a 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -22,6 +22,7 @@
 #include "miscadmin.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/memutils.h"
 
 /* Working state needed by gistbulkdelete */
@@ -44,8 +45,7 @@ typedef struct
 
 static void gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						   IndexBulkDeleteCallback callback, void *callback_state);
-static void gistvacuumpage(GistVacState *vstate, BlockNumber blkno,
-						   BlockNumber orig_blkno);
+static void gistvacuumpage(GistVacState *vstate, Buffer buffer);
 static void gistvacuum_delete_empty_pages(IndexVacuumInfo *info,
 										  GistVacState *vstate);
 static bool gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
@@ -129,8 +129,9 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	GistVacState vstate;
 	BlockNumber num_pages;
 	bool		needLock;
-	BlockNumber blkno;
 	MemoryContext oldctx;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -208,9 +209,17 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	blkno = GIST_ROOT_BLKNO;
+	p.current_blocknum = GIST_ROOT_BLKNO;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -219,12 +228,23 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 			UnlockRelationForExtension(rel, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
-			gistvacuumpage(&vstate, blkno, blkno);
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
+		{
+			gistvacuumpage(&vstate, buf);
+		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/*
 	 * If we found any recyclable pages (and recorded them in the FSM), then
@@ -269,15 +289,16 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * are recursing to re-examine a previous page).
  */
 static void
-gistvacuumpage(GistVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
+gistvacuumpage(GistVacState *vstate, Buffer buffer)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteCallback callback = vstate->callback;
 	void	   *callback_state = vstate->callback_state;
 	Relation	rel = info->index;
-	Buffer		buffer;
+	BlockNumber orig_blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 	BlockNumber recurse_to;
+	BlockNumber blkno = orig_blkno;
 
 restart:
 	recurse_to = InvalidBlockNumber;
@@ -285,9 +306,6 @@ restart:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-								info->strategy);
-
 	/*
 	 * We are not going to stay here for a long time, aggressively grab an
 	 * exclusive lock.
@@ -450,6 +468,8 @@ restart:
 	if (recurse_to != InvalidBlockNumber)
 	{
 		blkno = recurse_to;
+		buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+									info->strategy);
 		goto restart;
 	}
 }
-- 
2.39.5 (Apple Git-154)

From 7a5a52d298670ef8a88b455c0f05e25069510d43 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 13:53:39 +0300
Subject: [PATCH v7 3/3] Use read_stream in SP-GiST vacuum

---
 src/backend/access/spgist/spgvacuum.c | 38 ++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index 0da069f..7ae163c 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -25,6 +25,7 @@
 #include "storage/bufmgr.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/snapmgr.h"
 
 
@@ -618,17 +619,15 @@ vacuumRedirectAndPlaceholder(Relation index, Relation heaprel, Buffer buffer)
  * Process one page during a bulkdelete scan
  */
 static void
-spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
+spgvacuumpage(spgBulkDeleteState *bds, Buffer		buffer)
 {
 	Relation	index = bds->info->index;
-	Buffer		buffer;
+	BlockNumber blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	buffer = ReadBufferExtended(index, MAIN_FORKNUM, blkno,
-								RBM_NORMAL, bds->info->strategy);
 	LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 	page = (Page) BufferGetPage(buffer);
 
@@ -805,8 +804,10 @@ spgvacuumscan(spgBulkDeleteState *bds)
 {
 	Relation	index = bds->info->index;
 	bool		needLock;
-	BlockNumber num_pages,
-				blkno;
+	BlockNumber num_pages;
+	Buffer		buf;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/* Finish setting up spgBulkDeleteState */
 	initSpGistState(&bds->spgstate, index);
@@ -833,7 +834,14 @@ spgvacuumscan(spgBulkDeleteState *bds)
 	 * delete some deletable tuples.  See more extensive comments about this
 	 * in btvacuumscan().
 	 */
-	blkno = SPGIST_METAPAGE_BLKNO + 1;
+	p.current_blocknum = SPGIST_METAPAGE_BLKNO + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										bds->info->strategy,
+										index,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -844,17 +852,27 @@ spgvacuumscan(spgBulkDeleteState *bds)
 			UnlockRelationForExtension(index, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
+
+		p.last_exclusive = num_pages;
+
 		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			spgvacuumpage(bds, blkno);
+			spgvacuumpage(bds, buf);
 			/* empty the pending-list after each page */
 			if (bds->pendingList != NULL)
 				spgprocesspending(bds);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Propagate local lastUsedPages cache to metablock */
 	SpGistUpdateMetaPage(index);
-- 
2.39.5 (Apple Git-154)

From f34aa92fa93cf492828d5931958e6f8075acfcbb Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v8 1/3] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
---
 src/backend/access/nbtree/nbtree.c            | 70 +++++++++++-----
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/007_vacuum_btree.pl   | 79 +++++++++++++++++++
 3 files changed, 129 insertions(+), 21 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/007_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index dd76fe1da90..0849ad6d819 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -31,6 +31,7 @@
 #include "storage/lmgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -85,7 +86,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static BlockNumber btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -984,8 +985,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	Relation	rel = info->index;
 	BTVacState	vstate;
 	BlockNumber num_pages;
-	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1054,9 +1056,17 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	scanblkno = BTREE_METAPAGE + 1;
+	p.current_blocknum = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -1069,17 +1079,31 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 										 num_pages);
 
 		/* Quit if we've scanned the whole relation */
-		if (scanblkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; scanblkno < num_pages; scanblkno++)
+
+		/* In 007_vacuum_btree test we need to coordinate two distinguishable points here */
+		INJECTION_POINT("nbtree-vacuum-1");
+		INJECTION_POINT("nbtree-vacuum-2");
+
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			btvacuumpage(&vstate, scanblkno);
+			BlockNumber current_block = btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
+											 current_block);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1109,9 +1133,11 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * after our cycleid was acquired) whose right half page happened to reuse
  * a block that we might have processed at some point before it was
  * recycled (i.e. before the page split).
+ *
+ * Returns BlockNumber of a scanned page (not backtracked).
  */
-static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+static BlockNumber
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1122,7 +1148,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1136,14 +1162,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1179,7 +1197,7 @@ backtrack:
 					 errmsg_internal("right sibling %u of scanblkno %u unexpectedly in an inconsistent state in index \"%s\"",
 									 blkno, scanblkno, RelationGetRelationName(rel))));
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 
 		/*
@@ -1199,7 +1217,7 @@ backtrack:
 		{
 			/* Done with current scanblkno (and all lower split pages) */
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 	}
 
@@ -1430,8 +1448,18 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
+	return scanblkno;
 }
 
 /*
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 283ffa751aa..a456a5e77f8 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -15,6 +15,7 @@ tests += {
       't/004_io_direct.pl',
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
+      't/007_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/007_vacuum_btree.pl b/src/test/modules/test_misc/t/007_vacuum_btree.pl
new file mode 100644
index 00000000000..2c69d2b477d
--- /dev/null
+++ b/src/test/modules/test_misc/t/007_vacuum_btree.pl
@@ -0,0 +1,79 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies that B-tree vacuum can restart read stream.
+# To do so we need to insert some data during vacuum. So we wait in injection point
+# after first vacuum scan. During this wait we insert some data forcing page split.
+# this split will trigger relation extension and subsequent read_stream_reset().
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize postgres
+my $psql_err = '';
+my $psql_out = '';
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# This ensures autovacuum do not run
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# From this point, vacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-2', 'wait');");
+
+my $psql_session = $node->background_psql('postgres');
+
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		create table a as select random() r from generate_series(1,100) x;
+		create index on a(r);
+		delete from a;
+		vacuum (index_cleanup on) a;
+	));
+
+# Wait until an vacuum worker starts.
+$node->wait_for_event('client backend', 'nbtree-vacuum-2');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-1', 'wait');");
+
+# Here's the key point of a test: during vacuum we add some page splits.
+# This will force vacuum into doing another scan thus reseting read stream.
+$node->safe_psql('postgres',
+	"insert into a select x from generate_series(1,3000) x;");
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-2');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-2');");
+
+# Observe that second scan is reached.
+$node->wait_for_event('client backend', 'nbtree-vacuum-1');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-1');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-1');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.42.0

From 7b0e4b5d6b91b14c032cd5ec81fb4cc4ddf07894 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 13:53:39 +0300
Subject: [PATCH v8 3/3] Use read_stream in SP-GiST vacuum

---
 src/backend/access/spgist/spgvacuum.c | 38 ++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index 0da069fd4d7..7ae163c92dc 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -25,6 +25,7 @@
 #include "storage/bufmgr.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/snapmgr.h"
 
 
@@ -618,17 +619,15 @@ vacuumRedirectAndPlaceholder(Relation index, Relation heaprel, Buffer buffer)
  * Process one page during a bulkdelete scan
  */
 static void
-spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
+spgvacuumpage(spgBulkDeleteState *bds, Buffer		buffer)
 {
 	Relation	index = bds->info->index;
-	Buffer		buffer;
+	BlockNumber blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	buffer = ReadBufferExtended(index, MAIN_FORKNUM, blkno,
-								RBM_NORMAL, bds->info->strategy);
 	LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 	page = (Page) BufferGetPage(buffer);
 
@@ -805,8 +804,10 @@ spgvacuumscan(spgBulkDeleteState *bds)
 {
 	Relation	index = bds->info->index;
 	bool		needLock;
-	BlockNumber num_pages,
-				blkno;
+	BlockNumber num_pages;
+	Buffer		buf;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/* Finish setting up spgBulkDeleteState */
 	initSpGistState(&bds->spgstate, index);
@@ -833,7 +834,14 @@ spgvacuumscan(spgBulkDeleteState *bds)
 	 * delete some deletable tuples.  See more extensive comments about this
 	 * in btvacuumscan().
 	 */
-	blkno = SPGIST_METAPAGE_BLKNO + 1;
+	p.current_blocknum = SPGIST_METAPAGE_BLKNO + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										bds->info->strategy,
+										index,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -844,17 +852,27 @@ spgvacuumscan(spgBulkDeleteState *bds)
 			UnlockRelationForExtension(index, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
+
+		p.last_exclusive = num_pages;
+
 		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			spgvacuumpage(bds, blkno);
+			spgvacuumpage(bds, buf);
 			/* empty the pending-list after each page */
 			if (bds->pendingList != NULL)
 				spgprocesspending(bds);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Propagate local lastUsedPages cache to metablock */
 	SpGistUpdateMetaPage(index);
-- 
2.42.0

From ecf95f53bacbfa55c48c4e66d6e275c49a0555bb Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 10:00:49 +0300
Subject: [PATCH v8 2/3] Use read_stream in GiST vacuum

---
 src/backend/access/gist/gistvacuum.c | 46 ++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index 24fb94f473e..a86122a53d1 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -22,6 +22,7 @@
 #include "miscadmin.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/memutils.h"
 
 /* Working state needed by gistbulkdelete */
@@ -44,8 +45,7 @@ typedef struct
 
 static void gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						   IndexBulkDeleteCallback callback, void *callback_state);
-static void gistvacuumpage(GistVacState *vstate, BlockNumber blkno,
-						   BlockNumber orig_blkno);
+static void gistvacuumpage(GistVacState *vstate, Buffer buffer);
 static void gistvacuum_delete_empty_pages(IndexVacuumInfo *info,
 										  GistVacState *vstate);
 static bool gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
@@ -129,8 +129,9 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	GistVacState vstate;
 	BlockNumber num_pages;
 	bool		needLock;
-	BlockNumber blkno;
 	MemoryContext oldctx;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -208,9 +209,17 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	blkno = GIST_ROOT_BLKNO;
+	p.current_blocknum = GIST_ROOT_BLKNO;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -219,12 +228,23 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 			UnlockRelationForExtension(rel, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
-			gistvacuumpage(&vstate, blkno, blkno);
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
+		{
+			gistvacuumpage(&vstate, buf);
+		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/*
 	 * If we found any recyclable pages (and recorded them in the FSM), then
@@ -269,15 +289,16 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * are recursing to re-examine a previous page).
  */
 static void
-gistvacuumpage(GistVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
+gistvacuumpage(GistVacState *vstate, Buffer buffer)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteCallback callback = vstate->callback;
 	void	   *callback_state = vstate->callback_state;
 	Relation	rel = info->index;
-	Buffer		buffer;
+	BlockNumber orig_blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 	BlockNumber recurse_to;
+	BlockNumber blkno = orig_blkno;
 
 restart:
 	recurse_to = InvalidBlockNumber;
@@ -285,9 +306,6 @@ restart:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point();
 
-	buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-								info->strategy);
-
 	/*
 	 * We are not going to stay here for a long time, aggressively grab an
 	 * exclusive lock.
@@ -450,6 +468,8 @@ restart:
 	if (recurse_to != InvalidBlockNumber)
 	{
 		blkno = recurse_to;
+		buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+									info->strategy);
 		goto restart;
 	}
 }
-- 
2.42.0

From 4129b1164696ad5ca62a709728db6c5d9010a795 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Sat, 19 Oct 2024 12:46:29 +0500
Subject: [PATCH v9 1/4] Prototype B-tree vacuum streamlineing

Signed-off-by: Andrey Borodin <x4mmm@yandex-team.ru>
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
---
 src/backend/access/nbtree/nbtree.c            | 70 +++++++++++-----
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/008_vacuum_btree.pl   | 79 +++++++++++++++++++
 3 files changed, 129 insertions(+), 21 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/008_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index c0a8833e068..fe7084edacd 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -32,6 +32,7 @@
 #include "storage/lmgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -86,7 +87,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static BlockNumber btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -991,8 +992,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	Relation	rel = info->index;
 	BTVacState	vstate;
 	BlockNumber num_pages;
-	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1061,9 +1063,17 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	scanblkno = BTREE_METAPAGE + 1;
+	p.current_blocknum = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -1076,17 +1086,31 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 										 num_pages);
 
 		/* Quit if we've scanned the whole relation */
-		if (scanblkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; scanblkno < num_pages; scanblkno++)
+
+		/* In 007_vacuum_btree test we need to coordinate two distinguishable points here */
+		INJECTION_POINT("nbtree-vacuum-1");
+		INJECTION_POINT("nbtree-vacuum-2");
+
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			btvacuumpage(&vstate, scanblkno);
+			BlockNumber current_block = btvacuumpage(&vstate, buf);
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
+											 current_block);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
@@ -1116,9 +1140,11 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * after our cycleid was acquired) whose right half page happened to reuse
  * a block that we might have processed at some point before it was
  * recycled (i.e. before the page split).
+ *
+ * Returns BlockNumber of a scanned page (not backtracked).
  */
-static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+static BlockNumber
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1129,7 +1155,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1143,14 +1169,6 @@ backtrack:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point(false);
 
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1186,7 +1204,7 @@ backtrack:
 					 errmsg_internal("right sibling %u of scanblkno %u unexpectedly in an inconsistent state in index \"%s\"",
 									 blkno, scanblkno, RelationGetRelationName(rel))));
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 
 		/*
@@ -1206,7 +1224,7 @@ backtrack:
 		{
 			/* Done with current scanblkno (and all lower split pages) */
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 	}
 
@@ -1437,8 +1455,18 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
+		 * buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								info->strategy);
 		goto backtrack;
 	}
+	return scanblkno;
 }
 
 /*
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 9c50de7efb0..ed0822f19ee 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -16,6 +16,7 @@ tests += {
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
       't/007_catcache_inval.pl',
+      't/008_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/008_vacuum_btree.pl b/src/test/modules/test_misc/t/008_vacuum_btree.pl
new file mode 100644
index 00000000000..2c69d2b477d
--- /dev/null
+++ b/src/test/modules/test_misc/t/008_vacuum_btree.pl
@@ -0,0 +1,79 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies that B-tree vacuum can restart read stream.
+# To do so we need to insert some data during vacuum. So we wait in injection point
+# after first vacuum scan. During this wait we insert some data forcing page split.
+# this split will trigger relation extension and subsequent read_stream_reset().
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize postgres
+my $psql_err = '';
+my $psql_out = '';
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# This ensures autovacuum do not run
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# From this point, vacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-2', 'wait');");
+
+my $psql_session = $node->background_psql('postgres');
+
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		create table a as select random() r from generate_series(1,100) x;
+		create index on a(r);
+		delete from a;
+		vacuum (index_cleanup on) a;
+	));
+
+# Wait until an vacuum worker starts.
+$node->wait_for_event('client backend', 'nbtree-vacuum-2');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('nbtree-vacuum-1', 'wait');");
+
+# Here's the key point of a test: during vacuum we add some page splits.
+# This will force vacuum into doing another scan thus reseting read stream.
+$node->safe_psql('postgres',
+	"insert into a select x from generate_series(1,3000) x;");
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-2');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-2');");
+
+# Observe that second scan is reached.
+$node->wait_for_event('client backend', 'nbtree-vacuum-1');
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('nbtree-vacuum-1');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_wakeup('nbtree-vacuum-1');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.39.5 (Apple Git-154)

From 4a630aeb27f42a1be6b3d34007c3e5749373f09c Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 13:53:39 +0300
Subject: [PATCH v9 4/4] Use read_stream in SP-GiST vacuum

---
 src/backend/access/spgist/spgvacuum.c | 38 ++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index eeddacd0d52..f0d8428d909 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -25,6 +25,7 @@
 #include "storage/bufmgr.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/snapmgr.h"
 
 
@@ -618,17 +619,15 @@ vacuumRedirectAndPlaceholder(Relation index, Relation heaprel, Buffer buffer)
  * Process one page during a bulkdelete scan
  */
 static void
-spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
+spgvacuumpage(spgBulkDeleteState *bds, Buffer		buffer)
 {
 	Relation	index = bds->info->index;
-	Buffer		buffer;
+	BlockNumber blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point(false);
 
-	buffer = ReadBufferExtended(index, MAIN_FORKNUM, blkno,
-								RBM_NORMAL, bds->info->strategy);
 	LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 	page = (Page) BufferGetPage(buffer);
 
@@ -805,8 +804,10 @@ spgvacuumscan(spgBulkDeleteState *bds)
 {
 	Relation	index = bds->info->index;
 	bool		needLock;
-	BlockNumber num_pages,
-				blkno;
+	BlockNumber num_pages;
+	Buffer		buf;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/* Finish setting up spgBulkDeleteState */
 	initSpGistState(&bds->spgstate, index);
@@ -833,7 +834,14 @@ spgvacuumscan(spgBulkDeleteState *bds)
 	 * delete some deletable tuples.  See more extensive comments about this
 	 * in btvacuumscan().
 	 */
-	blkno = SPGIST_METAPAGE_BLKNO + 1;
+	p.current_blocknum = SPGIST_METAPAGE_BLKNO + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										bds->info->strategy,
+										index,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
 		/* Get the current relation length */
@@ -844,17 +852,27 @@ spgvacuumscan(spgBulkDeleteState *bds)
 			UnlockRelationForExtension(index, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
+
+		p.last_exclusive = num_pages;
+
 		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
 		{
-			spgvacuumpage(bds, blkno);
+			spgvacuumpage(bds, buf);
 			/* empty the pending-list after each page */
 			if (bds->pendingList != NULL)
 				spgprocesspending(bds);
 		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/* Propagate local lastUsedPages cache to metablock */
 	SpGistUpdateMetaPage(index);
-- 
2.39.5 (Apple Git-154)

From 96b7286f94b392af888d50df1a143dd9c03a9a53 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Thu, 24 Oct 2024 10:00:49 +0300
Subject: [PATCH v9 3/4] Use read_stream in GiST vacuum

---
 src/backend/access/gist/gistvacuum.c | 46 ++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index dd0d9d5006c..806bd0a4345 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -22,6 +22,7 @@
 #include "miscadmin.h"
 #include "storage/indexfsm.h"
 #include "storage/lmgr.h"
+#include "storage/read_stream.h"
 #include "utils/memutils.h"
 
 /* Working state needed by gistbulkdelete */
@@ -44,8 +45,7 @@ typedef struct
 
 static void gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						   IndexBulkDeleteCallback callback, void *callback_state);
-static void gistvacuumpage(GistVacState *vstate, BlockNumber blkno,
-						   BlockNumber orig_blkno);
+static void gistvacuumpage(GistVacState *vstate, Buffer buffer);
 static void gistvacuum_delete_empty_pages(IndexVacuumInfo *info,
 										  GistVacState *vstate);
 static bool gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
@@ -129,8 +129,9 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	GistVacState vstate;
 	BlockNumber num_pages;
 	bool		needLock;
-	BlockNumber blkno;
 	MemoryContext oldctx;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -208,9 +209,17 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	blkno = GIST_ROOT_BLKNO;
+	p.current_blocknum = GIST_ROOT_BLKNO;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer buf;
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -219,12 +228,23 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 			UnlockRelationForExtension(rel, ExclusiveLock);
 
 		/* Quit if we've scanned the whole relation */
-		if (blkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; blkno < num_pages; blkno++)
-			gistvacuumpage(&vstate, blkno, blkno);
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while(BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
+		{
+			gistvacuumpage(&vstate, buf);
+		}
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+		/*
+		 * After reaching the end we have to reset stream to use it again.
+		 * Extra restart in case of just one iteration does not cost us much.
+		 */
+		read_stream_reset(stream);
 	}
+	read_stream_end(stream);
 
 	/*
 	 * If we found any recyclable pages (and recorded them in the FSM), then
@@ -269,15 +289,16 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * are recursing to re-examine a previous page).
  */
 static void
-gistvacuumpage(GistVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
+gistvacuumpage(GistVacState *vstate, Buffer buffer)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteCallback callback = vstate->callback;
 	void	   *callback_state = vstate->callback_state;
 	Relation	rel = info->index;
-	Buffer		buffer;
+	BlockNumber orig_blkno = BufferGetBlockNumber(buffer);
 	Page		page;
 	BlockNumber recurse_to;
+	BlockNumber blkno = orig_blkno;
 
 restart:
 	recurse_to = InvalidBlockNumber;
@@ -285,9 +306,6 @@ restart:
 	/* call vacuum_delay_point while not holding any buffer lock */
 	vacuum_delay_point(false);
 
-	buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-								info->strategy);
-
 	/*
 	 * We are not going to stay here for a long time, aggressively grab an
 	 * exclusive lock.
@@ -450,6 +468,8 @@ restart:
 	if (recurse_to != InvalidBlockNumber)
 	{
 		blkno = recurse_to;
+		buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+									info->strategy);
 		goto restart;
 	}
 }
-- 
2.39.5 (Apple Git-154)

From aa7bf69a232c5adbb2dca791cd76ea7e92b15cd8 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Tue, 18 Mar 2025 23:11:14 +0500
Subject: [PATCH v9 2/4] Review fixes

---
 src/backend/access/nbtree/nbtree.c               | 13 ++++++++-----
 src/test/modules/test_misc/t/008_vacuum_btree.pl |  4 +---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index fe7084edacd..02688d90fe7 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -1089,7 +1089,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 		if (p.current_blocknum >= num_pages)
 			break;
 
-		/* In 007_vacuum_btree test we need to coordinate two distinguishable points here */
+		/* In 008_vacuum_btree test we need to coordinate two distinguishable points here */
 		INJECTION_POINT("nbtree-vacuum-1");
 		INJECTION_POINT("nbtree-vacuum-2");
 
@@ -1135,7 +1135,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * btvacuumpage --- VACUUM one page
  *
  * This processes a single page for btvacuumscan().  In some cases we must
- * backtrack to re-examine and VACUUM pages that were the scanblkno during
+ * backtrack to re-examine and VACUUM pages that were on buf's page during
  * a previous call here.  This is how we handle page splits (that happened
  * after our cycleid was acquired) whose right half page happened to reuse
  * a block that we might have processed at some point before it was
@@ -1166,9 +1166,6 @@ backtrack:
 	attempt_pagedel = false;
 	backtrack_to = P_NONE;
 
-	/* call vacuum_delay_point while not holding any buffer lock */
-	vacuum_delay_point(false);
-
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1456,6 +1453,9 @@ backtrack:
 	{
 		blkno = backtrack_to;
 
+		/* call vacuum_delay_point while not holding any buffer lock */
+		vacuum_delay_point(false);
+
 		/*
 		 * We can't use _bt_getbuf() here because it always applies
 		 * _bt_checkpage(), which will barf on an all-zero page. We want to
@@ -1466,6 +1466,9 @@ backtrack:
 								info->strategy);
 		goto backtrack;
 	}
+
+	/* call vacuum_delay_point while not holding any buffer lock */
+	vacuum_delay_point(false);
 	return scanblkno;
 }
 
diff --git a/src/test/modules/test_misc/t/008_vacuum_btree.pl b/src/test/modules/test_misc/t/008_vacuum_btree.pl
index 2c69d2b477d..8ec56c419a3 100644
--- a/src/test/modules/test_misc/t/008_vacuum_btree.pl
+++ b/src/test/modules/test_misc/t/008_vacuum_btree.pl
@@ -16,8 +16,6 @@ if ($ENV{enable_injection_points} ne 'yes')
 }
 
 # Initialize postgres
-my $psql_err = '';
-my $psql_out = '';
 my $node = PostgreSQL::Test::Cluster->new('node');
 $node->init;
 
@@ -47,7 +45,7 @@ $psql_session->query_until(
 		create table a as select random() r from generate_series(1,100) x;
 		create index on a(r);
 		delete from a;
-		vacuum (index_cleanup on) a;
+		vacuum (index_cleanup on, parallel 0) a;
 	));
 
 # Wait until an vacuum worker starts.
-- 
2.39.5 (Apple Git-154)

From 06689ada5011e1abeadaa0d33538e0b4069e5723 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Thu, 20 Mar 2025 16:09:41 -0400
Subject: [PATCH v10 2/2] test backtrack

---
 src/backend/access/nbtree/nbtree.c            |  4 +
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/008_vacuum_btree.pl   | 96 +++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/008_vacuum_btree.pl

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 353600414a2..be29c4e70e2 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -32,6 +32,7 @@
 #include "storage/lmgr.h"
 #include "utils/fmgrprotos.h"
 #include "utils/index_selfuncs.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -1178,6 +1179,7 @@ backtrack:
 	attempt_pagedel = false;
 	backtrack_to = P_NONE;
 
+	INJECTION_POINT("nbtree-vacuum-page");
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1468,6 +1470,8 @@ backtrack:
 		/* check for vacuum delay while not holding any buffer lock */
 		vacuum_delay_point(false);
 
+		INJECTION_POINT("nbtree-vacuum-page-backtrack");
+
 		/*
 		 * We can't use _bt_getbuf() here because it always applies
 		 * _bt_checkpage(), which will barf on an all-zero page. We want to
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 9c50de7efb0..ed0822f19ee 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -16,6 +16,7 @@ tests += {
       't/005_timeouts.pl',
       't/006_signal_autovacuum.pl',
       't/007_catcache_inval.pl',
+      't/008_vacuum_btree.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/008_vacuum_btree.pl b/src/test/modules/test_misc/t/008_vacuum_btree.pl
new file mode 100644
index 00000000000..d393e1809ce
--- /dev/null
+++ b/src/test/modules/test_misc/t/008_vacuum_btree.pl
@@ -0,0 +1,96 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# Test btree vacuum
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# Turn autovacuum off for the cluster. This could be done at the table level
+# instead. However, since this file exercises vacuum, turn off autovacuum
+# globally. This also allows use of non-local injection points in vacuum code.
+$node->append_conf('postgresql.conf', 'autovacuum = off');
+$node->start;
+
+# Check if extension injection_points is available
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+my $psql_session = $node->background_psql('postgres');
+
+# Create a table with an index filled with dead index entries
+$node->safe_psql('postgres', q[
+		create table test_backtrack (col1 int);
+		insert into test_backtrack select generate_series(1,800);
+		create index on test_backtrack(col1);
+		delete from test_backtrack;
+	]
+);
+
+# Attach to two injection points. The first one will allow us to stop between
+# vacuuming each index page. The second is our validation that we did backtrack.
+$psql_session->query_safe(
+	qq[
+    SELECT injection_points_set_local();
+    SELECT injection_points_attach('nbtree-vacuum-page', 'wait');
+    SELECT injection_points_attach('nbtree-vacuum-page-backtrack', 'wait');
+]);
+
+# Start a vacuum of the table and index.
+$psql_session->query_until(
+	qr/starting_bg_psql/,
+		q(\echo starting_bg_psql
+		vacuum (index_cleanup on, parallel 0) test_backtrack;
+	));
+
+# The index vacuum should be waiting on our first injection point and is yet to
+# process any pages.
+$node->wait_for_event('client backend', 'nbtree-vacuum-page');
+
+# Wake up vacuum so it can process the first index leaf page.
+$node->safe_psql('postgres', "SELECT injection_points_wakeup('nbtree-vacuum-page');");
+
+# The first index leaf page is now vacuumed and vacuum should be waiting again
+# on the first injection point.
+$node->wait_for_event('client backend', 'nbtree-vacuum-page');
+
+# Insert data while vacuum is waiting to process the next leaf page. The
+# inserted data will force a page split in which some tuples from unprocessed
+# leaf pages will be moved to the first already vacuumed leaf page.
+$node->safe_psql('postgres',
+	"insert into test_backtrack select generate_series(1,300);");
+
+# Now we want the vacuum to continue. We don't want to wait on our first break
+# point again.
+# We need to make sure we are waiting before detaching and issuing a wakeup.
+# Otherwise there could be a race and the backend may not get woken up.
+$node->wait_for_event('client backend', 'nbtree-vacuum-page');
+$node->safe_psql('postgres', "SELECT injection_points_detach('nbtree-vacuum-page');");
+$node->safe_psql('postgres', "SELECT injection_points_wakeup('nbtree-vacuum-page');");
+
+# Wait on our second break point. Vacuum should have been forced to backtrack
+# and vacuum the first leaf page again to ensure it removed all dead index
+# entries.
+$node->wait_for_event('client backend', 'nbtree-vacuum-page-backtrack');
+
+# Once we wait on our second break point, we're done. Time to tell the backend
+# to detach and wake it up.
+$node->safe_psql('postgres', "SELECT injection_points_detach('nbtree-vacuum-page-backtrack');");
+$node->safe_psql('postgres', "SELECT injection_points_wakeup('nbtree-vacuum-page-backtrack');");
+
+ok($psql_session->quit);
+
+done_testing();
-- 
2.34.1

From e558d78483d36ee510f13389029663e6bace9a27 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Thu, 20 Mar 2025 16:09:11 -0400
Subject: [PATCH v10 1/2] Use streaming read I/O in btree vacuuming

Btree vacuum processes all index pages in physical order. Now it uses
the read stream API to get the next buffer instead of explicitly
invoking ReadBuffer().

It is possible for concurrent indexes to cause page splits during index
vacuuming. This can lead to index entries that have yet to be vacuumed
being moved to pages that have already been vacuumed. Btree vacuum code
handles this by backtracking to reprocess those pages. So, while normal,
sequentially encountered pages are now read through the read stream API,
backtracked pages are still read with explicit ReadBuffer() calls.

Author: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/flat/CAAKRu_bW1UOyup%3DjdFw%2BkOF9bCaAm%3D9UpiyZtbPMn8n_vnP%2Big%40mail.gmail.com#3b3a84132fc683b3ee5b40bc4c2ea2a5
---
 src/backend/access/nbtree/nbtree.c | 91 ++++++++++++++++++++++--------
 1 file changed, 66 insertions(+), 25 deletions(-)

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index c0a8833e068..353600414a2 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -86,7 +86,7 @@ typedef struct BTParallelScanDescData *BTParallelScanDesc;
 static void btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 						 IndexBulkDeleteCallback callback, void *callback_state,
 						 BTCycleId cycleid);
-static void btvacuumpage(BTVacState *vstate, BlockNumber scanblkno);
+static BlockNumber btvacuumpage(BTVacState *vstate, Buffer buf);
 static BTVacuumPosting btreevacuumposting(BTVacState *vstate,
 										  IndexTuple posting,
 										  OffsetNumber updatedoffset,
@@ -991,8 +991,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	Relation	rel = info->index;
 	BTVacState	vstate;
 	BlockNumber num_pages;
-	BlockNumber scanblkno;
 	bool		needLock;
+	BlockRangeReadStreamPrivate p;
+	ReadStream *stream = NULL;
 
 	/*
 	 * Reset fields that track information about the entire index now.  This
@@ -1061,9 +1062,18 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 */
 	needLock = !RELATION_IS_LOCAL(rel);
 
-	scanblkno = BTREE_METAPAGE + 1;
+	p.current_blocknum = BTREE_METAPAGE + 1;
+	stream = read_stream_begin_relation(READ_STREAM_FULL,
+										info->strategy,
+										rel,
+										MAIN_FORKNUM,
+										block_range_read_stream_cb,
+										&p,
+										0);
 	for (;;)
 	{
+		Buffer		buf;
+
 		/* Get the current relation length */
 		if (needLock)
 			LockRelationForExtension(rel, ExclusiveLock);
@@ -1076,18 +1086,44 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 										 num_pages);
 
 		/* Quit if we've scanned the whole relation */
-		if (scanblkno >= num_pages)
+		if (p.current_blocknum >= num_pages)
 			break;
-		/* Iterate over pages, then loop back to recheck length */
-		for (; scanblkno < num_pages; scanblkno++)
+
+
+		p.last_exclusive = num_pages;
+
+		/* Iterate over pages, then loop back to recheck relation length */
+		while (true)
 		{
-			btvacuumpage(&vstate, scanblkno);
+			BlockNumber current_block;
+
+			/* call vacuum_delay_point while not holding any buffer lock */
+			vacuum_delay_point(false);
+
+			buf = read_stream_next_buffer(stream, NULL);
+
+			if (!BufferIsValid(buf))
+				break;
+
+			current_block = btvacuumpage(&vstate, buf);
+
 			if (info->report_progress)
 				pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
-											 scanblkno);
+											 current_block);
 		}
+
+		Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
+
+		/*
+		 * We have to reset the read stream to use it again. After returning
+		 * InvalidBuffer, the read stream API won't invoke our callback again
+		 * until the stream has been reset.
+		 */
+		read_stream_reset(stream);
 	}
 
+	read_stream_end(stream);
+
 	/* Set statistics num_pages field to final size of index */
 	stats->num_pages = num_pages;
 
@@ -1111,14 +1147,16 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
  * btvacuumpage --- VACUUM one page
  *
  * This processes a single page for btvacuumscan().  In some cases we must
- * backtrack to re-examine and VACUUM pages that were the scanblkno during
+ * backtrack to re-examine and VACUUM pages that were on buf's page during
  * a previous call here.  This is how we handle page splits (that happened
  * after our cycleid was acquired) whose right half page happened to reuse
  * a block that we might have processed at some point before it was
  * recycled (i.e. before the page split).
+ *
+ * Returns BlockNumber of a scanned page (not backtracked).
  */
-static void
-btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
+static BlockNumber
+btvacuumpage(BTVacState *vstate, Buffer buf)
 {
 	IndexVacuumInfo *info = vstate->info;
 	IndexBulkDeleteResult *stats = vstate->stats;
@@ -1129,7 +1167,7 @@ btvacuumpage(BTVacState *vstate, BlockNumber scanblkno)
 	bool		attempt_pagedel;
 	BlockNumber blkno,
 				backtrack_to;
-	Buffer		buf;
+	BlockNumber scanblkno = BufferGetBlockNumber(buf);
 	Page		page;
 	BTPageOpaque opaque;
 
@@ -1140,17 +1178,6 @@ backtrack:
 	attempt_pagedel = false;
 	backtrack_to = P_NONE;
 
-	/* call vacuum_delay_point while not holding any buffer lock */
-	vacuum_delay_point(false);
-
-	/*
-	 * We can't use _bt_getbuf() here because it always applies
-	 * _bt_checkpage(), which will barf on an all-zero page. We want to
-	 * recycle all-zero pages, not fail.  Also, we want to use a nondefault
-	 * buffer access strategy.
-	 */
-	buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
-							 info->strategy);
 	_bt_lockbuf(rel, buf, BT_READ);
 	page = BufferGetPage(buf);
 	opaque = NULL;
@@ -1186,7 +1213,7 @@ backtrack:
 					 errmsg_internal("right sibling %u of scanblkno %u unexpectedly in an inconsistent state in index \"%s\"",
 									 blkno, scanblkno, RelationGetRelationName(rel))));
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 
 		/*
@@ -1206,7 +1233,7 @@ backtrack:
 		{
 			/* Done with current scanblkno (and all lower split pages) */
 			_bt_relbuf(rel, buf);
-			return;
+			return scanblkno;
 		}
 	}
 
@@ -1437,8 +1464,22 @@ backtrack:
 	if (backtrack_to != P_NONE)
 	{
 		blkno = backtrack_to;
+
+		/* check for vacuum delay while not holding any buffer lock */
+		vacuum_delay_point(false);
+
+		/*
+		 * We can't use _bt_getbuf() here because it always applies
+		 * _bt_checkpage(), which will barf on an all-zero page. We want to
+		 * recycle all-zero pages, not fail.  Also, we want to use a
+		 * nondefault buffer access strategy.
+		 */
+		buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
+								 info->strategy);
 		goto backtrack;
 	}
+
+	return scanblkno;
 }
 
 /*
-- 
2.34.1