From 0f7d58f7386d0e3a55bdcf931492b62ee883a998 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Tue, 10 Jun 2025 11:00:36 +0530 Subject: [PATCH 10/17] Reinitialize StrategyControl after resizing buffers The commit introduces a separate function StrategyReInitialize() instead of reusing StrategyInitialize() since some of the things that the second one does are not required in the first one. Here's list of what StrategyReInitialize() does and how does it differ from StrategyInitialize(). 1. When expanding the buffer pool add new buffers to the free list. 2. When shrinking buffers, we remove any buffers, in the area being shrunk, from the freelist. While doing so we adjust the first and last free buffer pointers in the StrategyControl area. Hence nothing more needed after resizing. 3. Check the sanity of the free buffer list is added after resizing. 4. StrategyControl pointer needn't be fetched again since it should not change. But added an Assert to make sure the pointer is valid. 5. &StrategyControl->buffer_strategy_lock need not be initialized again. 6. completePasses and numBufferAllocs need not be cleared since the server is still running and the previous statistics is still valid. TODO: Since StrategyControl plays a crucial role in background writer as well as clock tick algorithm, the impact of resizing the buffers on those two needs to be assessed. We may require more adjustments to those two as well as StrategyControl based on the assessment. --- src/backend/storage/buffer/buf_init.c | 11 ++- src/backend/storage/buffer/freelist.c | 120 ++++++++++++++++++++++++++ src/include/storage/buf_internals.h | 1 + 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c index f78be4700df..7b8bc577bd5 100644 --- a/src/backend/storage/buffer/buf_init.c +++ b/src/backend/storage/buffer/buf_init.c @@ -163,8 +163,15 @@ BufferManagerShmemInit(int FirstBufferToInit) if (FirstBufferToInit < NBuffers) GetBufferDescriptor(NBuffers - 1)->freeNext = FREENEXT_END_OF_LIST; - /* Init other shared buffer-management stuff */ - StrategyInitialize(!foundDescs); + /* + * Init other shared buffer-management stuff from scratch configuring buffer + * pool the first time. If we are just resizing buffer pool adjust only the + * required structures. + */ + if (FirstBufferToInit == 0) + StrategyInitialize(!foundDescs); + else + StrategyReInitialize(FirstBufferToInit); /* Initialize per-backend file flush context */ WritebackContextInit(&BackendWritebackContext, diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c index e384e46c779..c5277e090a5 100644 --- a/src/backend/storage/buffer/freelist.c +++ b/src/backend/storage/buffer/freelist.c @@ -98,6 +98,9 @@ static BufferDesc *GetBufferFromRing(BufferAccessStrategy strategy, uint32 *buf_state); static void AddBufferToRing(BufferAccessStrategy strategy, BufferDesc *buf); +#ifdef USE_ASSERT_CHECKING +static void StrategyValidateFreeList(void); +#endif /* USE_ASSERT_CHECKING */ /* * ClockSweepTick - Helper routine for StrategyGetBuffer() @@ -526,6 +529,75 @@ StrategyInitialize(bool init) Assert(!init); } +/* + * StrategyReInitialize -- re-initialize the buffer cache replacement + * strategy. + * + * To be called when resizing buffer manager and only from the coordinator. + * TODO: Assess the differences between this function and StrategyInitialize(). + */ +void +StrategyReInitialize(int FirstBufferIdToInit) +{ + bool found; + + /* + * Resizing memory for buffer pools should not affect the address of + * StrategyControl. + */ + if (StrategyControl != (BufferStrategyControl *) + ShmemInitStructInSegment("Buffer Strategy Status", + sizeof(BufferStrategyControl), + &found, STRATEGY_SHMEM_SEGMENT)) + elog(FATAL, "something went wrong while re-initializing the buffer strategy"); + + /* TODO: Buffer lookup table adjustment: There are two options: + * + * 1. Resize the buffer lookup table to match the new number of buffers. But + * this requires rehashing all the entries in the buffer lookup table with + * the new table size. + * + * 2. Allocate maximum size of the buffer lookup table at the beginning and + * never resize it. This leaves sparse buffer lookup table which is + * inefficient from both memory and time perspective. According to David + * Rowley, the sparse entries in the buffer look up table cause frequent + * cacheline reload which affect performance. If the impact of that + * inefficiency in a benchmark is significant, we will need to consider first + * option. + */ + + /* + * When shrinking buffers, we must have adjusted the first and the last free + * buffer when removing the buffers being shrunk from the free list. Nothing + * to be done here. + * + * When expanding the shared buffers, new buffers are added at the end of the + * freelist or they form the new free list if there are no free buffers. + */ + if (FirstBufferIdToInit < NBuffers) + { + if (StrategyControl->firstFreeBuffer == FREENEXT_END_OF_LIST) + StrategyControl->firstFreeBuffer = FirstBufferIdToInit; + else + { + Assert(StrategyControl->lastFreeBuffer >= 0); + GetBufferDescriptor(StrategyControl->lastFreeBuffer - 1)->freeNext = FirstBufferIdToInit; + } + + StrategyControl->lastFreeBuffer = NBuffers - 1; + } + + /* Check free list sanity after resizing. */ +#ifdef USE_ASSERT_CHECKING + StrategyValidateFreeList(); +#endif /* USE_ASSERT_CHECKING */ + + /* Initialize the clock sweep pointer */ + pg_atomic_init_u32(&StrategyControl->nextVictimBuffer, 0); + + /* No pending notification */ + StrategyControl->bgwprocno = -1; +} /* * StrategyPurgeFreeList -- remove all buffers with id higher than the number of @@ -595,6 +667,54 @@ StrategyPurgeFreeList(int numBuffers) */ } +#ifdef USE_ASSERT_CHECKING +/* + * StrategyValidateFreeList-- check sanity of free buffer list. + */ +static void +StrategyValidateFreeList(void) +{ + int nextFree = StrategyControl->firstFreeBuffer; + int numFreeBuffers = 0; + int lastFreeBuffer = FREENEXT_END_OF_LIST; + + SpinLockAcquire(&StrategyControl->buffer_strategy_lock); + + while (nextFree != FREENEXT_END_OF_LIST) + { + BufferDesc *buf = GetBufferDescriptor(nextFree); + + /* nextFree should be id of buffer being examined. */ + Assert(nextFree == buf->buf_id); + Assert(buf->buf_id < NBuffers); + /* The buffer should not be marked as not in the list. */ + Assert(buf->freeNext != FREENEXT_NOT_IN_LIST); + + /* Update our knowledge of last buffer in the free list. */ + lastFreeBuffer = buf->buf_id; + + numFreeBuffers++; + + /* Avoid infinite recursion in case there are cycles in free list. */ + if (numFreeBuffers > NBuffers) + break; + + nextFree = buf->freeNext; + } + + Assert(numFreeBuffers <= NBuffers); + + /* + * Make sure that the StrategyControl's knowledge of last free buffer + * agrees with what's there in the free list. + */ + if (StrategyControl->firstFreeBuffer != FREENEXT_END_OF_LIST) + Assert(StrategyControl->lastFreeBuffer == lastFreeBuffer); + + SpinLockRelease(&StrategyControl->buffer_strategy_lock); +} +#endif /* USE_ASSERT_CHECKING */ + /* ---------------------------------------------------------------- * Backend-private buffer ring management * ---------------------------------------------------------------- diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index add15e3723b..46949e9d90e 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -454,6 +454,7 @@ extern void StrategyNotifyBgWriter(int bgwprocno); extern Size StrategyShmemSize(void); extern void StrategyInitialize(bool init); extern void StrategyPurgeFreeList(int numBuffers); +extern void StrategyReInitialize(int FirstBufferToInit); extern bool have_free_buffer(void); /* buf_table.c */ -- 2.34.1