Re: Changing shared_buffers without restart

Hi Yuhang,

Sorry for the late reply. Was working through the patches in order.

On Fri, 11 Sept 2026 at 03:56, Yuhang Qiu <iamqyh(at)gmail(dot)com> wrote:
>
> Hi Palak,
>
> > Yes. v20260908-0016 rechecks BM_TAG_VALID under LockBufHdr() and does
> > UnlockBufHdr() + continue instead of asserting when the tag was
> > cleared concurrently.
> >
> > Right, that line predates the MADV_REMOVE range fix. Removed it in
> > v20260908-0015.
>
> Thanks. 0015 and 0016 LGTM.

Thanks.

> I also tested the patches and did some more review.
>
> With shared_buffers=32MB and max_shared_buffers left at its default,
parallel
> workers fail to start, even without any resize:
> FATAL: failed to initialize shared_buffers to 16384
> CONTEXT: parallel worker
>
> RestoreGUCState() resets the GUC to its 128MB boot value, which exceeds
> MaxNBuffers and fails the check hook.

Reproduced this with shared_buffers=32MB and max_shared_buffers left at
its default. Fixed in 0015 by allowing the temporary PGC_S_DEFAULT
assignment only while InitializingParallelWorker is set.

Also checked removing the last configured value. Allowing PGC_S_DEFAULT
unconditionally accepts the 128MB default above the reserved 32MB
maximum, followed by a PANIC on resize. The revised check rejects this
reset.

Added test 008 for both cases. All nine assertions pass, including
launching two workers. Without the fix, it fails with the reported
initialization error. It also fails with the unconditional
default-source exception.

> There is also a performance issue in the grow path:
> PGSharedMemoryEnsureAllocated() gets the whole structure's range, rather
than
> just the added range. Even a small increase makes MADV_POPULATE_WRITE
walk the
> existing range again, adding overhead for large buffer pools. Could we
limit
> this to the page-aligned [current_end, new_end), as in the shrink path?

Changed this in 0013 to populate only the newly added pages. Also did
the testing. For a 512MB to 528MB grow, seven untraced runs gave a median
resize time of 332.7ms with the old range and 11.4ms with the new range.
Repeated the optimized run after the control and got 11.4ms again.

This was on Linux 6.17 with huge_pages=off, no swap and no concurrent
workload. These are total resize times for this case. Existing pages
that have been swapped out will be brought back when accessed. I have
not measured the effect under memory pressure with swap enabled.

> Two minor points:
> * check_shared_buffers() allows equality with MaxNBuffers, so "must be
less than"
> should be "must not exceed".
> * In proc.c, "specifid there" should be "specified there".

Changed "must be less than" to "must not exceed" in 0015 and corrected
"specifid" to "specified" in 0016.

> Some issues raised in earlier reviews remain unaddressed. Could you
include a
> status list with the next update, separating resolved and unresolved
items?
> That would help reviewers who are new to the thread or haven't followed
it for
> a while.

Included the complete series and renumbered it as v20260917-0001
through 0019. The numbers below refer to this new set. Patches 0001-0010
are Ashutosh's base commits, including his fixups, with authorship
preserved. Patches 0011-0019 are my earlier fixes and the current
follow-ups. No earlier attachments need to be applied separately.

Also went through the TODOs. Below is what is done and what remains.
File and line references are after applying all 19 patches.

Done:

- 0011: Included the earlier pg_buffercache fix to process one buffer
at a time in pg_buffercache_os_pages.

- 0012: Recompute MaxProportionalPins when a backend processes a resize
barrier, so the pin limit follows the new pool size.

- 0013: Retained the corrected MADV_REMOVE shrink range and included
the grow-range change discussed above. Also included the configure
declaration checks for the madvise constants.

- 0014: Check BM_TAG_VALID during eviction. A buffer can already have
a lookup-table entry while its read is still in progress, so checking
only whether its contents are valid can miss it. Recheck under the
header lock and roll back if eviction fails. Test 006 checks rollback,
reader completion and successful retry.
003 already covers the fully-valid pinned-buffer case.

- 0015: Fixed the parallel-worker initialization and configuration-reset
cases discussed above.

- 0016: Covered the TODO to document unsupported platforms. The
documentation now says that the resize function reports an error
when resizable shared memory is unavailable. Also repaired the
garbled paragraph and the typo.
See doc/src/sgml/func/func-admin.sgml:163.

- 0016: Covered the numeric memory-overhead example TODO. Verified with
max_shared_buffers=1GB and 8kB blocks on the 64-bit build. The lookup
table and checkpoint sort array together allocate 9,976,780 bytes,
about 9.5MB. Got the same result with shared_buffers=32MB and 128MB.
See doc/src/sgml/config.sgml:1875.

- 0017: Covered the TODO to validate hints inside ReadRecentBuffer().
A buffer number saved before shrink may be outside the new pool.
The function now rejects it before accessing the descriptor,
allowing the caller to do a normal lookup.
See src/backend/storage/buffer/bufmgr.c:843.

- 0017: Covered the TODO to check pg_shmem_allocations after rollback.
Test 001 now compares allocation metadata for all five buffer-manager
structures instead of checking only buffer counts. Also disabled
huge pages so rounding does not hide the small allocation changes.
All 414 assertions pass. This checks allocation metadata, not whether
physical memory was released.
See src/test/buffermgr/t/001_resize_fault_tolerance.pl:290.
The snapshot helper is at line 48; huge_pages=off is at line 29.

- 0018: Included the earlier SIGHUP test-ordering fix from
v20260817-0009. Start the resizer after configuration reload so the
expansion-failure test does not attempt resize with the old target.

- 0019: Included the earlier mprotect diagnostic fix from
v20260908-0011. The messages distinguish failure to make the active
range writable from failure to protect the reserved tail.

Not done:

- The variable-naming changes raised earlier are still pending.

- Direct stale-hint coverage for 0017. The code check is included, but
there is no test that keeps a buffer-number hint across a shrink
which removes that buffer slot. Such a test should check that the
old hint is rejected and the caller finds the page by normal lookup.
The rollback tests do not exercise this case.

- SHOW shared_buffers still includes the pending target, so its output
cannot be passed to pg_size_bytes(). Should SHOW return a parseable
size and leave the pending target to pg_get_buffer_resize_status()?

- A failed grow may leave some pages allocated in the unused range.
MADV_POPULATE_WRITE can fail after populating part of the range, and
making it inaccessible again does not free those pages. I have not
tested this failure case or added cleanup for it.

Thanks,
Palak