From 44debab4e13fe84bec3cc8ce7cb4ea8d971cfd43 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Wed, 5 Aug 2026 18:16:45 +0530 Subject: [PATCH] squash! Follow-up changes since last email on hackers Fix apw_dump_now() to not assume that the buffer pool is fixed sized. --- contrib/pg_prewarm/autoprewarm.c | 32 ++++++++++++++----- src/test/buffermgr/t/016_stress_pg_prewarm.pl | 3 -- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index dd202894b4e..2150d82f19c 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -675,6 +675,7 @@ static int apw_dump_now(bool is_bgworker, bool dump_unlogged) { int num_blocks; + int max_blocks; int i; int ret; BlockInfoRecord *block_info_array; @@ -702,26 +703,35 @@ apw_dump_now(bool is_bgworker, bool dump_unlogged) return 0; } - /* - * TODO: we need to modify this function to not rely on NBuffers being - * constant. - */ - /* * With sufficiently large shared_buffers, allocation will exceed 1GB, so - * allow for a huge allocation to prevent outright failure. + * allow for a huge allocation to prevent outright failure. Use the current + * size of the buffer pool as the estimate of the number of blocks to dump, + * and grow the array if necessary. * * (In the future, it might be a good idea to redesign this to use a more * memory-efficient data structure.) */ + max_blocks = NBuffers; block_info_array = (BlockInfoRecord *) - palloc_extended((sizeof(BlockInfoRecord) * NBuffers), MCXT_ALLOC_HUGE); + palloc_extended((sizeof(BlockInfoRecord) * max_blocks), MCXT_ALLOC_HUGE); for (num_blocks = 0, i = 0; i < NBuffers; i++) { uint64 buf_state; - CHECK_FOR_INTERRUPTS(); + /* + * Expand the array if necessary using the latest size of the buffer + * pool as the estimate of the number of blocks to dump. + */ + if (num_blocks >= max_blocks) + { + max_blocks = NBuffers; + block_info_array = (BlockInfoRecord *) + repalloc_extended(block_info_array, + sizeof(BlockInfoRecord) * max_blocks, + MCXT_ALLOC_HUGE); + } bufHdr = GetBufferDescriptor(i); @@ -747,6 +757,12 @@ apw_dump_now(bool is_bgworker, bool dump_unlogged) } UnlockBufHdr(bufHdr); + + /* + * Check for interrupts here, at the end of the loop, so that the buffer + * index i remains valid till the next iteration. + */ + CHECK_FOR_INTERRUPTS(); } snprintf(transient_dump_file_path, MAXPGPATH, "%s.tmp", AUTOPREWARM_FILE); diff --git a/src/test/buffermgr/t/016_stress_pg_prewarm.pl b/src/test/buffermgr/t/016_stress_pg_prewarm.pl index 23161f8865a..39b44191595 100644 --- a/src/test/buffermgr/t/016_stress_pg_prewarm.pl +++ b/src/test/buffermgr/t/016_stress_pg_prewarm.pl @@ -1,9 +1,6 @@ # Copyright (c) 2025-2026, PostgreSQL Global Development Group # # Stress test the autoprewarm concurrently with shared_buffers resizing. -# -# TODO: This test fails because apw_dump_now() assumes NBuffers is -# constant. Fix is on the way. use strict; use warnings;