From 5227f71f94fd28867ad52499c57ea3d875fcbf07 Mon Sep 17 00:00:00 2001 From: Zhong ShiHao Date: Wed, 16 Sep 2026 19:55:23 -0400 Subject: [PATCH v1] Let smgrtruncate() pass the known fork sizes to DropRelationBuffers() DropRelationBuffers() can avoid scanning the whole buffer pool when it knows the size of every fork, but it took that size from smgrnblocks_cached(), which returns a value only during recovery. So on a primary the targeted path was never used, and every truncation, most often the one done by vacuum, scanned the entire buffer pool while holding AccessExclusiveLock, no matter how few pages it removed. smgrtruncate() already receives the current fork sizes from its callers as old_nblocks, because it runs in a critical section and cannot measure them itself. Pass those sizes on to DropRelationBuffers(). This is safe because nothing can extend the relation between the measurement and the buffer drop. Outside recovery the caller holds AccessExclusiveLock, as smgrtruncate() already requires, and during recovery only the startup process extends relations. During recovery the supplied sizes are the same values the smgr cache would have given, so behavior there does not change. Also add an assertion to RelationTruncate() that the caller holds AccessExclusiveLock, since the buffer drop now depends on it. --- src/backend/catalog/storage.c | 7 ++++++ src/backend/storage/buffer/bufmgr.c | 38 ++++++++++++++++++++++++----- src/backend/storage/smgr/smgr.c | 2 +- src/include/storage/bufmgr.h | 3 ++- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index e443a4993c5..d037184334c 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -30,6 +30,7 @@ #include "pgstat.h" #include "storage/bulk_write.h" #include "storage/freespace.h" +#include "storage/lmgr.h" #include "storage/proc.h" #include "storage/smgr.h" #include "utils/hsearch.h" @@ -297,6 +298,12 @@ RelationTruncate(Relation rel, BlockNumber nblocks) int nforks = 0; SMgrRelation reln; + /* + * DropRelationBuffers() relies on the fork sizes we measure below staying + * valid until the buffers are dropped, which needs this lock. + */ + Assert(CheckRelationLockedByMe(rel, AccessExclusiveLock, false)); + /* * Make sure smgr_targblock etc aren't pointing somewhere past new end. * (Note: don't rely on this reln pointer below this loop.) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 5c82865a084..91a3965b776 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4782,11 +4782,17 @@ BufferGetLSNAtomic(Buffer buffer) * later. It is also the responsibility of higher-level code to ensure * that no other process could be trying to load more pages of the * relation into buffers. + * + * If nForkBlocks is not NULL, it gives the current size of each fork, + * and the caller must make sure that no fork can grow before we are + * done. If it is NULL, the sizes are taken from the smgr cache. If + * the size of any fork is unknown, the whole buffer pool is scanned. * -------------------------------------------------------------------- */ void DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, - int nforks, BlockNumber *firstDelBlock) + int nforks, BlockNumber *nForkBlocks, + BlockNumber *firstDelBlock) { int i; int j; @@ -4815,10 +4821,27 @@ DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, * otherwise the background writer or checkpointer can lead to a PANIC * error while flushing buffers corresponding to files that don't exist. * - * To know the exact size, we rely on the size cached for each fork by us - * during recovery which limits the optimization to recovery and on - * standbys but we can easily extend it once we have shared cache for - * relation size. + * A size that is too large would only cost some extra lookups, but a size + * that is too small would leave buffers behind. + * + * If the caller supplied the sizes, use them. smgrtruncate() does so, + * after measuring the forks just before calling us. Nothing can extend + * the relation in between. Outside recovery the caller holds + * AccessExclusiveLock, and during recovery only the startup process + * extends relations. During recovery the supplied sizes are the same + * values that the smgr cache would give us. + * + * Buffers can still exist beyond the measured size if an earlier attempt + * to extend the relation failed, see ExtendBufferedRelShared(). Those + * buffers are neither valid nor dirty, so nobody will try to write them, + * and it is fine to leave them for the next extension to reuse. Reading + * past the end of the file with zero_damaged_pages can leave a valid but + * clean buffer there too, but that setting is only meant for recovering + * from corruption. + * + * Otherwise we fall back on the size cached for each fork, which is only + * trustworthy during recovery, so the optimization is limited to recovery + * and standbys. * * In recovery, we cache the value returned by the first lseek(SEEK_END) * and the future writes keeps the cached value up-to-date. See @@ -4831,7 +4854,10 @@ DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, for (i = 0; i < nforks; i++) { /* Get the number of blocks for a relation's fork */ - nForkBlock[i] = smgrnblocks_cached(smgr_reln, forkNum[i]); + if (nForkBlocks != NULL) + nForkBlock[i] = nForkBlocks[i]; + else + nForkBlock[i] = smgrnblocks_cached(smgr_reln, forkNum[i]); if (nForkBlock[i] == InvalidBlockNumber) { diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 5391640d861..d563dfc6f21 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -881,7 +881,7 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, * Get rid of any buffers for the about-to-be-deleted blocks. bufmgr will * just drop them without bothering to write the contents. */ - DropRelationBuffers(reln, forknum, nforks, nblocks); + DropRelationBuffers(reln, forknum, nforks, old_nblocks, nblocks); /* * Send a shared-inval message to force other backends to close any smgr diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index 6837b35fc6d..76433fda5d2 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -301,7 +301,8 @@ extern void CreateAndCopyRelationData(RelFileLocator src_rlocator, extern void FlushDatabaseBuffers(Oid dbid); extern void DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, - int nforks, BlockNumber *firstDelBlock); + int nforks, BlockNumber *nForkBlocks, + BlockNumber *firstDelBlock); extern void DropRelationsAllBuffers(SMgrRelation *smgr_reln, int nlocators); extern void DropDatabaseBuffers(Oid dbid); -- 2.37.1 (Apple Git-137.1)