Re: Changing shared_buffers without restart

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org, Heikki Linnakangas <heikki(dot)linnakangas(at)databricks(dot)com>, Haoyu Huang <haoyu(dot)huang(at)databricks(dot)com>
Cc: Tomas Vondra <tomas(at)vondra(dot)me>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, chaturvedipalak1911(at)gmail(dot)com, Andres Freund <andres(at)anarazel(dot)de>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
Subject: Re: Changing shared_buffers without restart
Date: 2026-07-24 12:56:12
Message-ID: CAExHW5tYUYY13b4S1fv-zdxNsEEJzTy1EW6mRUj+FFEpA4Q0=g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Fri, Feb 13, 2026 at 5:22 PM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Thu, Feb 12, 2026 at 7:43 PM Jakub Wartak
> <jakub(dot)wartak(at)enterprisedb(dot)com> wrote:
> >
> >
> > TBH, I haven't really looked at the code outside of that region, I'm just
> > trespasser that was interested in memfd ;)
>
> Your trespassing has been very helpful. I have started a separate
> thread to discuss resizable shared structures at [1]. Once the
> implementation there is somewhat finalized, it will be good to try
> your huge page tests again.

Here's the next version of the patch implementing shared buffer pool
resizing. The patch is based on the latest master. Here's the summary
of changes since the last version:

1. The patch now uses the new shared memory infrastructure that was
introduced in PG 19. Patch 0006 enhances that infrastructure to
support resizable shared structures. I will also post the same patch
to [1]. I am fine to discuss the patch in that thread or here. The
APIs for registering and resizing the structures are documented in the
programming interface documentation.

2. Patch 0007 implements the shared buffer pool resizing using the
resizable shared structures infrastructure. It has a lot of code
improvements, including better documentation in comments, READMEs,
user-facing documentation and more TAP tests. The
storage/buffer/README has a section on buffer resizing. The buffer
resizing is implemented in buf_resize.c, which also has detailed
comments about the implementation. I suggest starting the review with
the user documentation, README and buf_resize.c.

Both patches have detailed commit messages and also list open items
for discussion and TODOs. Opinions and suggestions on those are
welcome.

The remaining patches are:

0001 - Adds sanity Asserts in the background writer code; see the
commit message. This could be committed separately.
0002 - Decouples the use of the NBuffers variable as a GUC from its
use as the size of the shared buffer pool. Prepares for the buffer
pool size to differ from the GUC value, as required by the resizing
feature.
0003 - Adds a diagnostic view to show the contents of the buffer
lookup table. Useful for debugging and testing, but not necessarily
for final commit.
0004 - Small change to pass use_units down to the per-GUC show
callbacks. Required if we accept the new output of the SHOW
shared_buffers command.
0005 - Adds backend_pid as a member of background_psql (rationale in
the commit message). Used by the buffer pool resizing tests.

Haoyu shared his version of the buffer pool resizing patch in [2].
What follows is a review of that patch. That thread is for the
resizable shared memory work, whereas these patches are for shared
buffer pool resizing. Hence, I am replying here.

The patch isn't applicable as-is since it does not use the new shared
memory infrastructure. But I have picked up some ideas and code as
explained below.

1. Buffer-manager infrastructure: a two-water-mark scheme
(lowNBuffers/highNBuffers) protected by AccessNBuffersLock, a new

Shared / local variable mapping in my patch:

- shared activeNBuffers (was lowNBuffers in your patch) <-> local activeNBuffers
- shared currentNBuffers (was highNBuffers in your patch) <-> local NBuffers

The locals are shadows of the shared values, kept in sync via the
ProcSignalBarrier mechanism; detailed comments are in the patch.

I should probably have named the shared and local versions of the
"current" value the same, but the names have stuck. Once we decide the
final names, I will change them to be consistent. I would prefer
allocNBuffers and NBuffers respectively, but I am open to suggestions.

* enable_dynamic_shared_buffers (PGC_POSTMASTER): off by default; when
off, all of the new code paths are no-ops and the server behaves as
before.

If max_shared_buffers = shared_buffers OR if the buffer pool
structures are registered as fixed-size, the feature is disabled. I
don't think an additional GUC is needed.

Patch rebased onto upstream master from the v18-based development
branch.

@@ -88,16 +89,84 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)

The latest version of this function adapts the scan to the current
number of buffers. My patch does not touch this function at all.

+ BEGIN_NBUFFERS_ACCESS(localNBuffers);

This will block buffer pool resizing while a scan is in progress and
vice versa. Specifically, the changes in CheckPointBuffers() are
problematic. This function runs for minutes when there are several
buffers to sync. A resize will need to wait for that long. Instead, my
patch takes a different approach which does not require either of them
to wait too long. Please let me know what you think of that approach.
Another problem with this approach is that it adds a few steps to
acquire the lock which are unnecessary when no resize is in progress.
Since resizing is a rare event, I think we should penalize resizing
rather than normal buffer pool operations.

I think we need to fix pg_buffercache_os_pages_internal() to not rely
on NBuffers being constant throughout the duration of the function
execution. That's true for all the functions that allocate memory
before scanning the buffer pool. I have added TODOs in the current
patches to all such places. But I might have missed some. Please let
me know if you find any such places. We should first decide between
blocking buffer pool resizing while a scan is in progress, the
approach taken by my patch, or some different approach. Once we decide
that, we will fix all such places accordingly.

@@ -967,19 +976,29 @@ CheckpointerShmemRequest(void *arg)
{

The changes here seem to be specific to Neon. In my patch the requests
array is sized to the initial size of the buffer pool, capped by
MAX_CHECKPOINT_REQUESTS. Alternatively, we can size it to MaxNBuffers,
but still cap it to MAX_CHECKPOINT_REQUESTS to avoid wasting too much
memory. What do you think about that?

Size size;
+ size = offsetof(CheckpointerShmemStruct, requests);
+
/*
- * The size of the requests[] array is arbitrarily set equal to NBuffers.
- * But there is a cap of MAX_CHECKPOINT_REQUESTS to prevent accumulating
- * too many checkpoint requests in the ring buffer.
+ * The size of the requests[] array is arbitrarily set equal to the
+ * initial size of buffer pool. But there is a cap of
+ * MAX_CHECKPOINT_REQUESTS to prevent accumulating too many checkpoint
+ * requests in the ring buffer.
+ *
+ * Under dynamic_shared_buffers we use a small fixed cap instead --
+ * sizing the queue on MaxNBuffers would waste a lot of shmem under
+ * auto-scale, but a real (non-zero) queue is still required so that
+ * SYNC_UNLINK_REQUEST can be forwarded to the checkpointer for delayed
+ * unlink processing.
*/
- size = offsetof(CheckpointerShmemStruct, requests);
- size = add_size(size, mul_size(Min(NBuffers,
- MAX_CHECKPOINT_REQUESTS),
- sizeof(CheckpointerRequest)));
- ShmemRequestStruct(.name = "Checkpointer Data",
- .size = size,
- .ptr = (void **) &CheckpointerShmem,
- );
+ if (enable_dynamic_shared_buffers)
+ size = add_size(size, mul_size(DSB_CHECKPOINT_REQUESTS,
+ sizeof(CheckpointerRequest)));
+ else
+ size = add_size(size, mul_size(Min(NBuffersGUC,
+ MAX_CHECKPOINT_REQUESTS),
+ sizeof(CheckpointerRequest)));
+
+ return size;
}
diff --git a/src/backend/storage/buffer/buf_init.c
b/src/backend/storage/buffer/buf_init.c
index 1407c930c56..757c00d03d6 100644
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -14,12 +14,18 @@
*/
#include "postgres.h"
+#include <unistd.h>
+#ifdef __linux__
+#include <sys/mman.h>
+#endif
+
+#include "miscadmin.h"
#include "storage/aio.h"
#include "storage/buf_internals.h"
#include "storage/bufmgr.h"
-#include "storage/proclist.h"
+#include "storage/pg_shmem.h"
#include "storage/shmem.h"
-#include "storage/subsystems.h"
+#include "utils/memdebug.h"
BufferDescPadded *BufferDescriptors;
char *BufferBlocks;
@@ -69,6 +75,208 @@ const ShmemCallbacks BufferManagerShmemCallbacks = {
* multiple times. Check the PrivateRefCount infrastructure in bufmgr.c.
*/
+static Size
+buffer_pool_madvise_alignment(void)
... snip ...
+static Size
+BufferPoolArrayPhysicalExpand(void *baseptr, Size elem_size,
+ int lowNBuffers, int highNBuffers,
+ bool *success)
... snip ...
+static Size
+BufferPoolArrayPhysicalShrink(void *baseptr, Size elem_size,
+ int lowNBuffers, int highNBuffers,
+ bool *success)
... snip ...
@@ -76,26 +284,52 @@ const ShmemCallbacks BufferManagerShmemCallbacks = {
static void
BufferManagerShmemRequest(void *arg)
{

and other changes to buf_init.c
... snip ...
These changes are not required in buf_init.c. With the resizable
shared structures infrastructure, the memory allocation and
deallocation is handled in shmem.c. Please review those changes and
let me know if I am missing something.

+
+ /*
+ * Reset the clock-sweep cursor before lowering the low water mark. The
+ * existing cursor may point above new_size. Once we publish the new
+ * lowNBuffers, ClockSweepTick() may otherwise immediately wrap past
+ * the new buffers via modulo arithmetic. Resetting to 0 means the
+ * next sweep starts from the bottom of the surviving range.
+ */
+ StrategyReset(old_size, new_size);
+
+ elog(LOG, "[Shrink Barrier]: restricting allocations to %d buffers",
new_size);
+ INSTR_TIME_SET_CURRENT(phase_start);
+ /*
+ * Wait for all backends to acknowledge the new lowNBuffers. After the
+ * barrier returns, all new buffer allocations will land in [0, lowNBuffers)
+ * range. For buffers in [lowNBuffers, highNBuffers), backends can
+ * hold pins and create new pins on buffers already pinned.
+ * The EvictExtraBuffers() loop below will wait for all buffers in
+ * [lowNBuffers, highNBuffers) to be unpinned.
+ */
+ SharedBufferResizeBarrier(PROCSIGNAL_BARRIER_SHBUF_RESIZE,
CppAsString(PROCSIGNAL_BARRIER_SHBUF_RESIZE));

Two issues here. First, StrategyReset isn't bullet-proof:
nextVictimBuffer can wrap past the new size via concurrent allocations
while this function is still lowering lowNBuffers to new_size. Second,
it does nothing about the complete passes. In my patches I have
revised the reset to wrap nextVictimBuffer around and increment
complete passes when shrinking. Even so, we still need to reset the
background writer statistics, which I would like to avoid -- but I
think this is a step in the right direction.

+/*
+ * Cleanup callback. Runs from the transaction-abort PG_CATCH path *and* from
+ * before_shmem_exit() if the backend dies while holding the resize slot.
+ *
+ * Rollback policy:
+ * - Partial shrink (lowNBuffers < highNBuffers): restore lowNBuffers to
+ * highNBuffers so the buffer pool is consistent at the larger size.
+ * Memory for [lowNBuffers, highNBuffers) is still mapped, so rolling
+ * back is safe.
+ * - Partial expand: BufferManagerShmemExpand() may have populated some of
+ * [old_size, inflight_expand_target) without publishing the new water
+ * marks. This is wasteful but harmless. We surface a WARNING so operators
+ * know to retry the resize.
+ */
+static void
+ResetResizeInProgress(int code, Datum arg)
+{
... snip ...
+}

My patches currently resort to a server restart to recover from an
interrupted resize; I would like to improve on that. This function's
intentions seem to be to recover more gracefully. However, I think the
implementation is not bullet-proof. I have a TAP test which induces
all kinds of interruptions during resize. It will be good to see how
this implementation fares against that test. If you could provide an
incremental patch on top of my patches to implement a graceful
recovery, I will be happy to incorporate it into my patch after
review.

+
+ if (new_size < old_size)
+ DoShrink(rsinfo, old_size, new_size);
+ else
+ DoExpand(rsinfo, old_size, new_size);

In my earlier patches I didn't separate the C implementation function
and the actual resize function. But I see that it's useful now, and
have done it that way in the attached patches. I think I should write
separate functions for shrink and expand, as your patch does. I will
do that once the protocol is agreed upon.

I am also not sure whether it's useful to print the function progress
or statistics as the output of this function. A better approach might
be to plug into the progress tracker infrastructure. But resizing
isn't a long-running operation like vacuum or index build. So I am not
sure whether it is worth the effort.

+ static int saved_low_nbuffers = 0;
+ int current_low_nbuffers;
+
+ BEGIN_NBUFFERS_ACCESS(localNBuffers);

Locking the background writer for the duration of resizing means new
allocators may not find victims easily, and creates more work for the
checkpointer. Similarly, making resize wait for the background writer
to finish its work means resize may take longer. In my patch they do
not block each other. Like any other backend, the background writer
adapts to changes in the buffer pool through the ProcSignalBarrier
mechanism.

+
+ strategy_buf_id = StrategySyncStart(&strategy_passes, &recent_alloc,
+ &current_low_nbuffers);
+ if (current_low_nbuffers != saved_low_nbuffers)
+ {
+#ifdef BGW_DEBUG
+ elog(DEBUG2, "invalidated background writer state after pool resize:
%d -> %d buffers",
+ saved_low_nbuffers, current_low_nbuffers);
+#endif
+ saved_info_valid = false;
+ saved_low_nbuffers = current_low_nbuffers;
+ }

I have similar code in my patch, but I would like to avoid that as
explained above.

CheckPointBuffers(int flags)
{
- BufferSync(flags);
+ BEGIN_NBUFFERS_ACCESS(localNBuffers);
+ BufferSync(flags, localNBuffers);
+ END_NBUFFERS_ACCESS(localNBuffers);

As mentioned earlier, this could block buffer pool resizing for minutes.

@@ -5391,6 +5392,13 @@ ShowGUCOption(const struct config_generic
*record, bool use_units)
{
const struct config_int *conf = &record->_int;
+ /*
+ * Set NBuffersGUC here so that both SHOW shared_buffers (use_units==true)
+ * and pg_settings (use_units==false) reflect the current shared
buffer pool size.
+ */
+ if (conf->variable == &NBuffersGUC)
+ NBuffersGUC = GetLowNBuffers();
+

Should this be highNBuffers? That's the value that actually reflects
the current pool size -- lowNBuffers only equals it when no resize is
in progress. My patch instead prints both the current and the pending
size. Let me know what you think about that approach.

I didn't see any APIs to make the address range against the freed
shared memory inaccessible. That capability is added in the resizable
shared memory infrastructure.

The big missing piece in your patch is tests: there is nothing
exercising the resizing functionality itself or its synchronization
with other buffer pool operations. My patch has some, but I would like
to add more. Any help in that regard would be appreciated.

[1] https://www.postgresql.org/message-id/CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com
[2] https://www.postgresql.org/message-id/CAM1e6U5XDwKYZo6Jj3yD3xpCB4qkhRSQn8upauHt%3DWhEbK9VZA%40mail.gmail.com

--
Best Wishes,
Ashutosh Bapat

Attachment Content-Type Size
v20260724-0007-Allow-to-resize-shared-buffers-without-res.patch text/x-patch 178.8 KB
v20260724-0005-PID-of-the-backend-process-backing-the-Bac.patch text/x-patch 3.0 KB
v20260724-0003-Add-a-view-to-read-contents-of-shared-buff.patch text/x-patch 14.6 KB
v20260724-0006-Resizable-shared-memory-structures.patch text/x-patch 129.2 KB
v20260724-0004-Pass-use_units-parameter-to-GucShowHook-fu.patch text/x-patch 15.6 KB
v20260724-0002-Decouple-GUC-shared_buffers-and-size-of-th.patch text/x-patch 18.6 KB
v20260724-0001-Add-BgBufferSync-sanity-Asserts.patch text/x-patch 2.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2026-07-24 12:58:36 Re: Allow a condition string in an injection point
Previous Message vignesh C 2026-07-24 12:38:35 Re: sequencesync worker race with REFRESH SEQUENCES