Fixing pg_buffercache tests

contrib/pg_buffercache/Makefile

@@ -5,6 +5,9 @@ OBJS = \
 	$(WIN32RES) \
 	pg_buffercache_pages.o
 
+EXTRA_INSTALL=src/test/modules/injection_points \
+	contrib/test_decoding
+
 EXTENSION = pg_buffercache
 DATA = pg_buffercache--1.2.sql pg_buffercache--1.2--1.3.sql \
 	pg_buffercache--1.1--1.2.sql pg_buffercache--1.0--1.1.sql \
@@ -13,6 +16,7 @@ DATA = pg_buffercache--1.2.sql pg_buffercache--1.2--1.3.sql \
 PGFILEDESC = "pg_buffercache - monitoring of shared buffer cache in real-time"
 
 REGRESS = pg_buffercache pg_buffercache_numa
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config

contrib/pg_buffercache/expected/pg_buffercache.out

@@ -1,8 +1,9 @@
 CREATE EXTENSION pg_buffercache;
-select count(*) = (select setting::bigint
-                   from pg_settings
-                   where name = 'shared_buffers')
-from pg_buffercache;
+select setting::bigint AS nbuffers
+        from pg_settings
+        where name = 'shared_buffers'
+\gset
+select count(*) = :nbuffers from pg_buffercache;
  ?column? 
 ----------
  t
@@ -33,6 +34,20 @@ SELECT count(*) > 0 FROM pg_buffercache_usage_counts() WHERE buffers >= 0;
  t
 (1 row)
 
+-- Test the buffer lookup table function and count is <= shared_buffers
+select count(*) <= :nbuffers from pg_buffercache_lookup_table_entries();
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Check that pg_buffercache_lookup_table view works and count is <= shared_buffers
+select count(*) <= :nbuffers from pg_buffercache_lookup_table;
+ ?column? 
+----------
+ t
+(1 row)
+
 -- Check that the functions / views can't be accessed by default. To avoid
 -- having to create a dedicated user, use the pg_database_owner pseudo-role.
 SET ROLE pg_database_owner;
@@ -46,6 +61,10 @@ SELECT * FROM pg_buffercache_summary();
 ERROR:  permission denied for function pg_buffercache_summary
 SELECT * FROM pg_buffercache_usage_counts();
 ERROR:  permission denied for function pg_buffercache_usage_counts
+SELECT * FROM pg_buffercache_lookup_table_entries();
+ERROR:  permission denied for function pg_buffercache_lookup_table_entries
+SELECT * FROM pg_buffercache_lookup_table;
+ERROR:  permission denied for view pg_buffercache_lookup_table
 RESET role;
 -- Check that pg_monitor is allowed to query view / function
 SET ROLE pg_monitor;
@@ -73,6 +92,21 @@ SELECT count(*) > 0 FROM pg_buffercache_usage_counts();
  t
 (1 row)
 
+RESET role;
+-- Check that pg_read_all_stats is allowed to query buffer lookup table
+SET ROLE pg_read_all_stats;
+SELECT count(*) >= 0 FROM pg_buffercache_lookup_table_entries();
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT count(*) >= 0 FROM pg_buffercache_lookup_table;
+ ?column? 
+----------
+ t
+(1 row)
+
 RESET role;
 ------
 ---- Test pg_buffercache_evict* functions

contrib/pg_buffercache/meson.build

@@ -38,5 +38,13 @@ tests += {
       'pg_buffercache',
       'pg_buffercache_numa',
     ],
-  },
+   },
+  'tap': {
+    'env': {
+      'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+    },
+    'tests': [
+      't/001_basic.pl',
+    ],
+   }
 }

contrib/pg_buffercache/pg_buffercache--1.5--1.6.sql

@@ -44,3 +44,27 @@ CREATE FUNCTION pg_buffercache_evict_all(
     OUT buffers_skipped int4)
 AS 'MODULE_PATHNAME', 'pg_buffercache_evict_all'
 LANGUAGE C PARALLEL SAFE VOLATILE;
+
+-- Add the buffer lookup table function
+CREATE FUNCTION pg_buffercache_lookup_table_entries(
+    OUT tablespace oid,
+    OUT database oid,
+    OUT relfilenode oid,
+    OUT forknum int2,
+    OUT blocknum int8,
+    OUT bufferid int4)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pg_buffercache_lookup_table_entries'
+LANGUAGE C PARALLEL SAFE VOLATILE;
+
+-- Create a view for convenient access.
+CREATE VIEW pg_buffercache_lookup_table AS
+    SELECT * FROM pg_buffercache_lookup_table_entries();
+
+-- Don't want these to be available to public.
+REVOKE ALL ON FUNCTION pg_buffercache_lookup_table_entries() FROM PUBLIC;
+REVOKE ALL ON pg_buffercache_lookup_table FROM PUBLIC;
+
+-- Grant access to monitoring role.
+GRANT EXECUTE ON FUNCTION pg_buffercache_lookup_table_entries() TO pg_read_all_stats;
+GRANT SELECT ON pg_buffercache_lookup_table TO pg_read_all_stats;

contrib/pg_buffercache/pg_buffercache_pages.c

@@ -16,6 +16,8 @@
 #include "storage/buf_internals.h"
 #include "storage/bufmgr.h"
 #include "utils/rel.h"
+#include "utils/tuplestore.h"
+#include "utils/injection_point.h"
 
 
 #define NUM_BUFFERCACHE_PAGES_MIN_ELEM	8
@@ -101,6 +103,7 @@ PG_FUNCTION_INFO_V1(pg_buffercache_usage_counts);
 PG_FUNCTION_INFO_V1(pg_buffercache_evict);
 PG_FUNCTION_INFO_V1(pg_buffercache_evict_relation);
 PG_FUNCTION_INFO_V1(pg_buffercache_evict_all);
+PG_FUNCTION_INFO_V1(pg_buffercache_lookup_table_entries);
 
 
 /* Only need to touch memory once per backend process lifetime */
@@ -117,6 +120,7 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 	TupleDesc	tupledesc;
 	TupleDesc	expected_tupledesc;
 	HeapTuple	tuple;
+	int			currentNBuffers = pg_atomic_read_u32(&ShmemCtrl->currentNBuffers);
 
 	if (SRF_IS_FIRSTCALL())
 	{
@@ -173,10 +177,10 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		/* Allocate NBuffers worth of BufferCachePagesRec records. */
 		fctx->record = (BufferCachePagesRec *)
 			MemoryContextAllocHuge(CurrentMemoryContext,
-								   sizeof(BufferCachePagesRec) * NBuffers);
+								   sizeof(BufferCachePagesRec) * currentNBuffers);
 
 		/* Set max calls and remember the user function context. */
-		funcctx->max_calls = NBuffers;
+		funcctx->max_calls = currentNBuffers; 
 		funcctx->user_fctx = fctx;
 
 		/* Return to original context when allocating transient memory */
@@ -190,13 +194,29 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 		 * snapshot across all buffers, but we do grab the buffer header
 		 * locks, so the information of each buffer is self-consistent.
 		 */
-		for (i = 0; i < NBuffers; i++)
+
+		/* Injection point before starting scan to test resize interaction */
+		INJECTION_POINT("pg-buffercache-scan-start", NULL);
+
+		for (i = 0; i < currentNBuffers; i++)
 		{
 			BufferDesc *bufHdr;
 			uint32		buf_state;
 
 			CHECK_FOR_INTERRUPTS();
 
+			INJECTION_POINT("pg-buffercache-scan-loop", NULL);
+			/*
+			 * TODO: We should just scan the entire buffer descriptor
+			 * array instead of relying on curent buffer pool size. But that can
+			 * happen if only we setup the descriptor array large enough at the
+			 * server startup time.
+			 */
+			if (currentNBuffers != pg_atomic_read_u32(&ShmemCtrl->currentNBuffers))
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("number of shared buffers changed during scan of buffer cache")));
+
 			bufHdr = GetBufferDescriptor(i);
 			/* Lock each buffer header before inspecting. */
 			buf_state = LockBufHdr(bufHdr);
@@ -222,6 +242,10 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 				fctx->record[i].isvalid = false;
 
 			UnlockBufHdr(bufHdr);
+
+			/* Injection point mid-scan to test resize during iteration */
+			if (i == currentNBuffers / 2)
+				INJECTION_POINT("pg-buffercache-scan-middle", NULL);
 		}
 	}
 
@@ -330,6 +354,7 @@ pg_buffercache_os_pages_internal(FunctionCallInfo fcinfo, bool include_numa)
 		int			max_entries;
 		char	   *startptr,
 				   *endptr;
+		int			currentNBuffers = pg_atomic_read_u32(&ShmemCtrl->currentNBuffers);
 
 		/* If NUMA information is requested, initialize NUMA support. */
 		if (include_numa && pg_numa_init() == -1)
@@ -372,7 +397,7 @@ pg_buffercache_os_pages_internal(FunctionCallInfo fcinfo, bool include_numa)
 			startptr = (char *) TYPEALIGN_DOWN(os_page_size,
 											   BufferGetBlock(1));
 			endptr = (char *) TYPEALIGN(os_page_size,
-										(char *) BufferGetBlock(NBuffers) + BLCKSZ);
+										(char *) BufferGetBlock(currentNBuffers) + BLCKSZ);
 			os_page_count = (endptr - startptr) / os_page_size;
 
 			/* Used to determine the NUMA node for all OS pages at once */
@@ -398,7 +423,7 @@ pg_buffercache_os_pages_internal(FunctionCallInfo fcinfo, bool include_numa)
 			Assert(idx == os_page_count);
 
 			elog(DEBUG1, "NUMA: NBuffers=%d os_page_count=" UINT64_FORMAT " "
-				 "os_page_size=%zu", NBuffers, os_page_count, os_page_size);
+				 "os_page_size=%zu", currentNBuffers, os_page_count, os_page_size);
 
 			/*
 			 * If we ever get 0xff back from kernel inquiry, then we probably
@@ -447,7 +472,7 @@ pg_buffercache_os_pages_internal(FunctionCallInfo fcinfo, bool include_numa)
 		 * without reallocating memory.
 		 */
 		pages_per_buffer = Max(1, BLCKSZ / os_page_size) + 1;
-		max_entries = NBuffers * pages_per_buffer;
+		max_entries = currentNBuffers * pages_per_buffer;
 
 		/* Allocate entries for BufferCacheOsPagesRec records. */
 		fctx->record = (BufferCacheOsPagesRec *)
@@ -470,7 +495,7 @@ pg_buffercache_os_pages_internal(FunctionCallInfo fcinfo, bool include_numa)
 		 */
 		startptr = (char *) TYPEALIGN_DOWN(os_page_size, (char *) BufferGetBlock(1));
 		idx = 0;
-		for (i = 0; i < NBuffers; i++)
+		for (i = 0; i < currentNBuffers; i++)
 		{
 			char	   *buffptr = (char *) BufferGetBlock(i + 1);
 			BufferDesc *bufHdr;
@@ -481,6 +506,11 @@ pg_buffercache_os_pages_internal(FunctionCallInfo fcinfo, bool include_numa)
 
 			CHECK_FOR_INTERRUPTS();
 
+			if (currentNBuffers != pg_atomic_read_u32(&ShmemCtrl->currentNBuffers))
+				ereport(ERROR,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					errmsg("number of shared buffers changed during scan of buffer cache")));
+
 			bufHdr = GetBufferDescriptor(i);
 
 			/* Lock each buffer header before inspecting. */
@@ -602,17 +632,23 @@ pg_buffercache_summary(PG_FUNCTION_ARGS)
 	int32		buffers_dirty = 0;
 	int32		buffers_pinned = 0;
 	int64		usagecount_total = 0;
+	int			currentNBuffers = pg_atomic_read_u32(&ShmemCtrl->currentNBuffers);
 
 	if (get_call_result_type(fcinfo, NULL, &tupledesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
-	for (int i = 0; i < NBuffers; i++)
+	for (int i = 0; i < currentNBuffers; i++)
 	{
 		BufferDesc *bufHdr;
 		uint32		buf_state;
 
 		CHECK_FOR_INTERRUPTS();
 
+		if (currentNBuffers != pg_atomic_read_u32(&ShmemCtrl->currentNBuffers))
+			ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("number of shared buffers changed during scan of buffer cache")));
+
 		/*
 		 * This function summarizes the state of all headers. Locking the
 		 * buffer headers wouldn't provide an improved result as the state of
@@ -664,17 +700,23 @@ pg_buffercache_usage_counts(PG_FUNCTION_ARGS)
 	int			pinned[BM_MAX_USAGE_COUNT + 1] = {0};
 	Datum		values[NUM_BUFFERCACHE_USAGE_COUNTS_ELEM];
 	bool		nulls[NUM_BUFFERCACHE_USAGE_COUNTS_ELEM] = {0};
+	int			currentNBuffers = pg_atomic_read_u32(&ShmemCtrl->currentNBuffers);
 
 	InitMaterializedSRF(fcinfo, 0);
 
-	for (int i = 0; i < NBuffers; i++)
+	for (int i = 0; i < currentNBuffers; i++)
 	{
 		BufferDesc *bufHdr = GetBufferDescriptor(i);
 		uint32		buf_state = pg_atomic_read_u32(&bufHdr->state);
 		int			usage_count;
 
 		CHECK_FOR_INTERRUPTS();
 
+		if (currentNBuffers != pg_atomic_read_u32(&ShmemCtrl->currentNBuffers))
+			ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("number of shared buffers changed during scan of buffer cache")));
+
 		usage_count = BUF_STATE_GET_USAGECOUNT(buf_state);
 		usage_counts[usage_count]++;
 
@@ -725,13 +767,15 @@ pg_buffercache_evict(PG_FUNCTION_ARGS)
 
 	Buffer		buf = PG_GETARG_INT32(0);
 	bool		buffer_flushed;
+	int			currentNBuffers = pg_atomic_read_u32(&ShmemCtrl->currentNBuffers);
+
 
 	if (get_call_result_type(fcinfo, NULL, &tupledesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
 	pg_buffercache_superuser_check("pg_buffercache_evict");
 
-	if (buf < 1 || buf > NBuffers)
+	if (buf < 1 || buf > currentNBuffers)
 		elog(ERROR, "bad buffer ID: %d", buf);
 
 	values[0] = BoolGetDatum(EvictUnpinnedBuffer(buf, &buffer_flushed));
@@ -826,3 +870,19 @@ pg_buffercache_evict_all(PG_FUNCTION_ARGS)
 
 	PG_RETURN_DATUM(result);
 }
+
+/*
+ * Return lookup table content as a set of records.
+ */
+Datum
+pg_buffercache_lookup_table_entries(PG_FUNCTION_ARGS)
+{
+	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+	InitMaterializedSRF(fcinfo, 0);
+
+	/* Fill the tuplestore */
+	BufTableGetContents(rsinfo->setResult, rsinfo->setDesc);
+
+	return (Datum) 0;
+}