agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
[PATCH 1/3] stress test for repack concurrently
3+ messages / 2 participants
[nested] [flat]

* [PATCH 1/3] stress test for repack concurrently
@ 2025-12-13 17:13  Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
  0 siblings, 0 replies; 3+ messages in thread

From: Mikhail Nikalayeu @ 2025-12-13 17:13 UTC (permalink / raw)

---
 contrib/amcheck/meson.build       |   1 +
 contrib/amcheck/t/007_repack_1.pl | 121 ++++++++++++++++++++++++++++++
 doc/src/sgml/regress.sgml         |  16 ++++
 3 files changed, 138 insertions(+)
 create mode 100644 contrib/amcheck/t/007_repack_1.pl

diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build
index d5137ef691d..cb4bc32e98a 100644
--- a/contrib/amcheck/meson.build
+++ b/contrib/amcheck/meson.build
@@ -50,6 +50,7 @@ tests += {
       't/004_verify_nbtree_unique.pl',
       't/005_pitr.pl',
       't/006_verify_gin.pl',
+      't/007_repack_1.pl',
     ],
   },
 }
diff --git a/contrib/amcheck/t/007_repack_1.pl b/contrib/amcheck/t/007_repack_1.pl
new file mode 100644
index 00000000000..17f3caa3a38
--- /dev/null
+++ b/contrib/amcheck/t/007_repack_1.pl
@@ -0,0 +1,121 @@
+# Copyright (c) 2021-2026, PostgreSQL Global Development Group
+
+# Test REPACK CONCURRENTLY with concurrent modifications
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+# PG_TEST_EXTRA carries a numerical stress value; 0 disables the test,
+# 1 means the test takes about 6 seconds, and the increase should be
+# roughly linear.
+my $matched = ($ENV{PG_TEST_EXTRA} // '') =~ /\bstress_concurrently(?:=(?<stressval>[0-9]*))?\b/;
+my $stressval = $+{stressval} // 1;
+if (!$matched or $stressval == 0)
+{
+	plan skip_all => 'skipping disabled REPACK CONCURRENTLY stress test';
+}
+
+note "stressval is $stressval";
+
+my $node;
+
+# Test set-up
+$node = PostgreSQL::Test::Cluster->new('CIC_test');
+$node->init;
+$node->append_conf('postgresql.conf',
+	'lock_timeout = ' . (1000 * $PostgreSQL::Test::Utils::timeout_default));
+$node->append_conf(
+	'postgresql.conf', qq(
+wal_level = logical
+));
+
+my $nrows = 10_000 * $stressval;
+my $duration = 6 * $stressval;
+my $no_hot = int(rand(2));
+
+$node->start;
+$node->safe_psql('postgres', q(CREATE TABLE tbl(id int PRIMARY KEY, val int)));
+
+if ($no_hot)
+{
+	$node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(val);));
+}
+else
+{
+	$node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(id);));
+}
+
+
+# Load amcheck
+$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
+
+# Insert $nrows rows into tbl
+$node->safe_psql('postgres', qq(
+	INSERT INTO tbl SELECT g, g FROM generate_series(1, $nrows) g
+));
+
+my $sum = $node->safe_psql('postgres', q(
+	SELECT SUM(val) AS sum FROM tbl
+));
+
+
+$node->pgbench(
+"--no-vacuum --client=30 --jobs=4 --exit-on-abort -T $duration",
+0,
+[qr{actually processed}],
+[qr{^$}],
+'concurrent operations with REINDEX/CREATE INDEX CONCURRENTLY',
+{
+	'concurrent_ops' => qq(
+		SELECT pg_try_advisory_lock(42)::integer AS gotlock \\gset
+		\\if :gotlock
+			REPACK (CONCURRENTLY) tbl USING INDEX tbl_pkey;
+			SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+			SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+			\\sleep 10 ms
+
+			REPACK (CONCURRENTLY) tbl USING INDEX test_idx;
+			SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+			SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+			\\sleep 10 ms
+
+			REPACK (CONCURRENTLY) tbl;
+			SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+			SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+			\\sleep 10 ms
+
+			SELECT pg_advisory_unlock(42);
+		\\else
+			\\set num_a random(1, $nrows)
+			\\set num_b random(1, $nrows)
+			\\set diff random(1, 10000)
+			BEGIN;
+			UPDATE tbl SET val = val + :diff WHERE id = :num_a;
+			\\sleep 1 ms
+			UPDATE tbl SET val = val - :diff WHERE id = :num_b;
+			\\sleep 1 ms
+			COMMIT;
+
+			BEGIN
+			--TRANSACTION ISOLATION LEVEL REPEATABLE READ
+			;
+			SELECT 1;
+			\\sleep 1 ms
+			SELECT COALESCE(SUM(val), 0) AS sum FROM tbl \\gset p_
+			\\if :p_sum != $sum
+				COMMIT;
+				SELECT (:p_sum) / 0;
+			\\endif
+
+			COMMIT;
+		\\endif
+	)
+});
+
+$node->stop;
+
+done_testing();
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index c74941bfbf2..c5b1b79a60c 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -386,6 +386,22 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption'
      </listitem>
     </varlistentry>
 
+    <varlistentry>
+     <term><literal>stress_concurrently</literal><optional>=VALUE</optional></term>
+     <listitem>
+      <para>
+       Run some additional, moderately expensive tests for commands with a
+       <literal>CONCURRENTLY</literal> option.  Optionally, an integer can be
+       given after an equal sign, which affects how long each such test
+       runs for.
+       Each test is calibrated so that, when given a value of 10,
+       it runs for approximately 60 seconds.
+       If no value is given, 1 is assumed.
+       Explicitly giving a value of 0 causes the test to be skipped.
+      </para>
+     </listitem>
+    </varlistentry>
+
     <varlistentry>
      <term><literal>wal_consistency_checking</literal></term>
      <listitem>
-- 
2.47.3


--tbdehtstrqwksfdh
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0002-one-more-stress-test-for-repack-concurrently.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* [PATCH v8 6/9] Make buffer hit helper
@ 2026-01-23 18:54  Melanie Plageman <melanieplageman@gmail.com>
  0 siblings, 0 replies; 3+ messages in thread

From: Melanie Plageman @ 2026-01-23 18:54 UTC (permalink / raw)

Already two places count buffer hits, requiring quite a few lines of
code since we do accounting in so many places. Future commits will add
more locations, so refactor into a helper.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/zljergweqti7x67lg5ije2rzjusie37nslsnkjkkby4laqqbfw@3p3zu522yykv
---
 src/backend/storage/buffer/bufmgr.c | 84 ++++++++++++++---------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index ab9c2a4b904..fa85570a791 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -648,6 +648,11 @@ static inline BufferDesc *BufferAlloc(SMgrRelation smgr,
 									  bool *foundPtr, IOContext io_context);
 static bool AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress);
 static void CheckReadBuffersOperation(ReadBuffersOperation *operation, bool is_complete);
+
+static pg_attribute_always_inline void TrackBufferHit(IOObject io_object,
+													  IOContext io_context,
+													  Relation rel, char persistence, SMgrRelation smgr,
+													  ForkNumber forknum, BlockNumber blocknum);
 static Buffer GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context);
 static void FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
 								IOObject io_object, IOContext io_context);
@@ -1243,18 +1248,14 @@ PinBufferForBlock(Relation rel,
 									   smgr->smgr_rlocator.backend);
 
 	if (persistence == RELPERSISTENCE_TEMP)
-	{
 		bufHdr = LocalBufferAlloc(smgr, forkNum, blockNum, foundPtr);
-		if (*foundPtr)
-			pgBufferUsage.local_blks_hit++;
-	}
 	else
-	{
 		bufHdr = BufferAlloc(smgr, persistence, forkNum, blockNum,
 							 strategy, foundPtr, io_context);
-		if (*foundPtr)
-			pgBufferUsage.shared_blks_hit++;
-	}
+
+	if (*foundPtr)
+		TrackBufferHit(io_object, io_context, rel, persistence, smgr, forkNum, blockNum);
+
 	if (rel)
 	{
 		/*
@@ -1263,21 +1264,6 @@ PinBufferForBlock(Relation rel,
 		 * zeroed instead), the per-relation stats always count them.
 		 */
 		pgstat_count_buffer_read(rel);
-		if (*foundPtr)
-			pgstat_count_buffer_hit(rel);
-	}
-	if (*foundPtr)
-	{
-		pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1, 0);
-		if (VacuumCostActive)
-			VacuumCostBalance += VacuumCostPageHit;
-
-		TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
-										  smgr->smgr_rlocator.locator.spcOid,
-										  smgr->smgr_rlocator.locator.dbOid,
-										  smgr->smgr_rlocator.locator.relNumber,
-										  smgr->smgr_rlocator.backend,
-										  true);
 	}
 
 	return BufferDescriptorGetBuffer(bufHdr);
@@ -1712,6 +1698,37 @@ ReadBuffersCanStartIO(Buffer buffer, bool nowait)
 	return ReadBuffersCanStartIOOnce(buffer, nowait);
 }
 
+/*
+ * We track various stats related to buffer hits. Because this is done in a
+ * few separate places, this helper exists for convenience.
+ */
+static pg_attribute_always_inline void
+TrackBufferHit(IOObject io_object, IOContext io_context,
+			   Relation rel, char persistence, SMgrRelation smgr,
+			   ForkNumber forknum, BlockNumber blocknum)
+{
+	TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum,
+									  blocknum,
+									  smgr->smgr_rlocator.locator.spcOid,
+									  smgr->smgr_rlocator.locator.dbOid,
+									  smgr->smgr_rlocator.locator.relNumber,
+									  smgr->smgr_rlocator.backend,
+									  true);
+
+	if (persistence == RELPERSISTENCE_TEMP)
+		pgBufferUsage.local_blks_hit += 1;
+	else
+		pgBufferUsage.shared_blks_hit += 1;
+
+	pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1, 0);
+
+	if (VacuumCostActive)
+		VacuumCostBalance += VacuumCostPageHit;
+
+	if (rel)
+		pgstat_count_buffer_hit(rel);
+}
+
 /*
  * Helper for WaitReadBuffers() that processes the results of a readv
  * operation, raising an error if necessary.
@@ -2007,25 +2024,8 @@ AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress)
 		 * must have started out as a miss in PinBufferForBlock(). The other
 		 * backend will track this as a 'read'.
 		 */
-		TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, blocknum,
-										  operation->smgr->smgr_rlocator.locator.spcOid,
-										  operation->smgr->smgr_rlocator.locator.dbOid,
-										  operation->smgr->smgr_rlocator.locator.relNumber,
-										  operation->smgr->smgr_rlocator.backend,
-										  true);
-
-		if (persistence == RELPERSISTENCE_TEMP)
-			pgBufferUsage.local_blks_hit += 1;
-		else
-			pgBufferUsage.shared_blks_hit += 1;
-
-		if (operation->rel)
-			pgstat_count_buffer_hit(operation->rel);
-
-		pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1, 0);
-
-		if (VacuumCostActive)
-			VacuumCostBalance += VacuumCostPageHit;
+		TrackBufferHit(io_object, io_context, operation->rel, persistence,
+					   operation->smgr, forknum, blocknum);
 	}
 	else
 	{
-- 
2.53.0.1.gb2826b52eb


--zousfcpkuk57oxzf
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v8-0007-Restructure-AsyncReadBuffers.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* [PATCH v7a 5/8] Make buffer hit helper
@ 2026-01-23 18:54  Melanie Plageman <melanieplageman@gmail.com>
  0 siblings, 0 replies; 3+ messages in thread

From: Melanie Plageman @ 2026-01-23 18:54 UTC (permalink / raw)

Already two places count buffer hits, requiring quite a few lines of
code since we do accounting in so many places. Future commits will add
more locations, so refactor into a helper.

Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/flat/zljergweqti7x67lg5ije2rzjusie37nslsnkjkkby4laqqbfw%403p3zu522yykv
---
 src/backend/storage/buffer/bufmgr.c | 84 ++++++++++++++---------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index ab9c2a4b904..fa85570a791 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -648,6 +648,11 @@ static inline BufferDesc *BufferAlloc(SMgrRelation smgr,
 									  bool *foundPtr, IOContext io_context);
 static bool AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress);
 static void CheckReadBuffersOperation(ReadBuffersOperation *operation, bool is_complete);
+
+static pg_attribute_always_inline void TrackBufferHit(IOObject io_object,
+													  IOContext io_context,
+													  Relation rel, char persistence, SMgrRelation smgr,
+													  ForkNumber forknum, BlockNumber blocknum);
 static Buffer GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context);
 static void FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
 								IOObject io_object, IOContext io_context);
@@ -1243,18 +1248,14 @@ PinBufferForBlock(Relation rel,
 									   smgr->smgr_rlocator.backend);
 
 	if (persistence == RELPERSISTENCE_TEMP)
-	{
 		bufHdr = LocalBufferAlloc(smgr, forkNum, blockNum, foundPtr);
-		if (*foundPtr)
-			pgBufferUsage.local_blks_hit++;
-	}
 	else
-	{
 		bufHdr = BufferAlloc(smgr, persistence, forkNum, blockNum,
 							 strategy, foundPtr, io_context);
-		if (*foundPtr)
-			pgBufferUsage.shared_blks_hit++;
-	}
+
+	if (*foundPtr)
+		TrackBufferHit(io_object, io_context, rel, persistence, smgr, forkNum, blockNum);
+
 	if (rel)
 	{
 		/*
@@ -1263,21 +1264,6 @@ PinBufferForBlock(Relation rel,
 		 * zeroed instead), the per-relation stats always count them.
 		 */
 		pgstat_count_buffer_read(rel);
-		if (*foundPtr)
-			pgstat_count_buffer_hit(rel);
-	}
-	if (*foundPtr)
-	{
-		pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1, 0);
-		if (VacuumCostActive)
-			VacuumCostBalance += VacuumCostPageHit;
-
-		TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
-										  smgr->smgr_rlocator.locator.spcOid,
-										  smgr->smgr_rlocator.locator.dbOid,
-										  smgr->smgr_rlocator.locator.relNumber,
-										  smgr->smgr_rlocator.backend,
-										  true);
 	}
 
 	return BufferDescriptorGetBuffer(bufHdr);
@@ -1712,6 +1698,37 @@ ReadBuffersCanStartIO(Buffer buffer, bool nowait)
 	return ReadBuffersCanStartIOOnce(buffer, nowait);
 }
 
+/*
+ * We track various stats related to buffer hits. Because this is done in a
+ * few separate places, this helper exists for convenience.
+ */
+static pg_attribute_always_inline void
+TrackBufferHit(IOObject io_object, IOContext io_context,
+			   Relation rel, char persistence, SMgrRelation smgr,
+			   ForkNumber forknum, BlockNumber blocknum)
+{
+	TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum,
+									  blocknum,
+									  smgr->smgr_rlocator.locator.spcOid,
+									  smgr->smgr_rlocator.locator.dbOid,
+									  smgr->smgr_rlocator.locator.relNumber,
+									  smgr->smgr_rlocator.backend,
+									  true);
+
+	if (persistence == RELPERSISTENCE_TEMP)
+		pgBufferUsage.local_blks_hit += 1;
+	else
+		pgBufferUsage.shared_blks_hit += 1;
+
+	pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1, 0);
+
+	if (VacuumCostActive)
+		VacuumCostBalance += VacuumCostPageHit;
+
+	if (rel)
+		pgstat_count_buffer_hit(rel);
+}
+
 /*
  * Helper for WaitReadBuffers() that processes the results of a readv
  * operation, raising an error if necessary.
@@ -2007,25 +2024,8 @@ AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress)
 		 * must have started out as a miss in PinBufferForBlock(). The other
 		 * backend will track this as a 'read'.
 		 */
-		TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, blocknum,
-										  operation->smgr->smgr_rlocator.locator.spcOid,
-										  operation->smgr->smgr_rlocator.locator.dbOid,
-										  operation->smgr->smgr_rlocator.locator.relNumber,
-										  operation->smgr->smgr_rlocator.backend,
-										  true);
-
-		if (persistence == RELPERSISTENCE_TEMP)
-			pgBufferUsage.local_blks_hit += 1;
-		else
-			pgBufferUsage.shared_blks_hit += 1;
-
-		if (operation->rel)
-			pgstat_count_buffer_hit(operation->rel);
-
-		pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1, 0);
-
-		if (VacuumCostActive)
-			VacuumCostBalance += VacuumCostPageHit;
+		TrackBufferHit(io_object, io_context, operation->rel, persistence,
+					   operation->smgr, forknum, blocknum);
 	}
 	else
 	{
-- 
2.53.0.1.gb2826b52eb


--iq4ka6vxlx6ycdfl
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v7a-0006-Restructure-AsyncReadBuffers.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-01-23 18:54 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-12-13 17:13 [PATCH 1/3] stress test for repack concurrently Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
2026-01-23 18:54 [PATCH v8 6/9] Make buffer hit helper Melanie Plageman <melanieplageman@gmail.com>
2026-01-23 18:54 [PATCH v7a 5/8] Make buffer hit helper Melanie Plageman <melanieplageman@gmail.com>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox