From 567f315d7ab7dd8f13916d7c31b8d3f3a1eef87f Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 17 Jun 2026 15:51:06 -0400 Subject: [PATCH v16 03/21] Inline BufferSync() into CheckPointBuffers() CheckPointBuffers() contained only the static function call to BufferSync(). It seemed more confusing than anything else. Inline the code of BufferSync(). --- src/backend/access/transam/xloginsert.c | 12 ++++---- src/backend/postmaster/checkpointer.c | 4 +-- src/backend/storage/buffer/bufmgr.c | 40 +++++++++---------------- src/backend/storage/sync/sync.c | 7 +++-- 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index c9aff944a2e..3a155eb483e 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -1142,12 +1142,12 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) /* * Update RedoRecPtr so that we can make the right decision. It's possible * that a new checkpoint will start just after GetRedoRecPtr(), but that - * is ok, as the buffer is already dirty, ensuring that any BufferSync() - * started after the buffer was marked dirty cannot complete without - * flushing this buffer. If a checkpoint started between marking the - * buffer dirty and this check, we will emit an unnecessary WAL record (as - * the buffer will be written out as part of the checkpoint), but the - * window for that is not big. + * is ok, as the buffer is already dirty, ensuring that any + * CheckPointBuffers() started after the buffer was marked dirty cannot + * complete without flushing this buffer. If a checkpoint started between + * marking the buffer dirty and this check, we will emit an unnecessary + * WAL record (as the buffer will be written out as part of the + * checkpoint), but the window for that is not big. */ RedoRecPtr = GetRedoRecPtr(); diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 580c7944119..b9b7145c4cd 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -781,8 +781,8 @@ FastCheckpointRequested(void) /* * CheckpointWriteDelay -- control rate of checkpoint * - * This function is called after each page write performed by BufferSync(). - * It is responsible for throttling BufferSync()'s write rate to hit + * This function is called after each page write performed by + * CheckPointBuffers(). It is responsible for throttling its write rate to hit * checkpoint_completion_target. * * The checkpoint request flags should be passed in; currently the only one diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 1ce4a689f4f..790a41e5e9c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -146,7 +146,7 @@ typedef struct PrivateRefCountEntry /* * Status of buffers to checkpoint for a particular tablespace, used - * internally in BufferSync. + * internally in CheckPointBuffers(). */ typedef struct CkptTsStatus { @@ -646,7 +646,6 @@ static bool PinBuffer(BufferDesc *buf, BufferUsageCountChange usage_count_change static void PinBuffer_Locked(BufferDesc *buf); static void UnpinBuffer(BufferDesc *buf); static void UnpinBufferNoOwner(BufferDesc *buf); -static void BufferSync(int flags); static int SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context); static void WaitIO(BufferDesc *buf); @@ -3587,7 +3586,7 @@ TrackNewBufferPin(Buffer buf) #include "lib/sort_template.h" /* - * BufferSync -- Write out all dirty buffers in the pool. + * Write out all dirty buffers in the buffer pool. * * This is called at checkpoint time to write out all dirty shared buffers. * The checkpoint request flags should be passed in. If CHECKPOINT_FAST is @@ -3595,9 +3594,12 @@ TrackNewBufferPin(Buffer buf) * CHECKPOINT_END_OF_RECOVERY or CHECKPOINT_FLUSH_UNLOGGED is set, we write * even unlogged buffers, which are otherwise skipped. The remaining flags * currently have no effect here. + * + * Note: temporary relations do not participate in checkpoints, so they don't + * need to be flushed. */ -static void -BufferSync(int flags) +void +CheckPointBuffers(int flags) { uint64 buf_state; int buf_id; @@ -3847,8 +3849,8 @@ BufferSync(int flags) } /* - * Issue all pending flushes. Only checkpointer calls BufferSync(), so - * IOContext will always be IOCONTEXT_NORMAL. + * Issue all pending flushes. Only the checkpointer calls + * CheckPointBuffers(), so IOContext will always be IOCONTEXT_NORMAL. */ IssuePendingWritebacks(&wb_context, IOCONTEXT_NORMAL); @@ -4469,20 +4471,6 @@ DebugPrintBufferRefcount(Buffer buffer) return result; } -/* - * CheckPointBuffers - * - * Flush all dirty blocks in buffer pool to disk at checkpoint time. - * - * Note: temporary relations do not participate in checkpoints, so they don't - * need to be flushed. - */ -void -CheckPointBuffers(int flags) -{ - BufferSync(flags); -} - /* * BufferGetBlockNumber * Returns the block number associated with a buffer. @@ -5796,11 +5784,11 @@ MarkSharedBufferDirtyHint(Buffer buffer, BufferDesc *bufHdr, uint64 lockstate, /* * We must mark the page dirty before we emit the WAL record, as per - * the usual rules, to ensure that BufferSync()/SyncOneBuffer() try to - * flush the buffer, even if we haven't inserted the WAL record yet. - * As we hold at least a share-exclusive lock, checkpoints will wait - * for this backend to be done with the buffer before continuing. If - * we did it the other way round, a checkpoint could start between + * the usual rules, to ensure that CheckPointBuffers()/BgBufferSync() + * try to flush the buffer, even if we haven't inserted the WAL record + * yet. As we hold at least a share-exclusive lock, checkpoints will + * wait for this backend to be done with the buffer before continuing. + * If we did it the other way round, a checkpoint could start between * writing the WAL record and marking the buffer dirty. */ buf_state = LockBufHdr(bufHdr); diff --git a/src/backend/storage/sync/sync.c b/src/backend/storage/sync/sync.c index 2c964b6f3d9..265dc1aca5b 100644 --- a/src/backend/storage/sync/sync.c +++ b/src/backend/storage/sync/sync.c @@ -313,9 +313,10 @@ ProcessSyncRequests(void) * requests that were queued by backends up to this point. The tightest * race condition that could occur is that a buffer that must be written * and fsync'd for the checkpoint could have been dumped by a backend just - * before it was visited by BufferSync(). We know the backend will have - * queued an fsync request before clearing the buffer's dirtybit, so we - * are safe as long as we do an Absorb after completing BufferSync(). + * before it was visited by CheckPointBuffers(). We know the backend will + * have queued an fsync request before clearing the buffer's dirtybit, so + * we are safe as long as we do an Absorb after completing + * CheckPointBuffers(). */ AbsorbSyncRequests(); -- 2.47.3