From 93d8b04e1e8c71110f66e34b45b672560f75112d Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Tue, 22 Dec 2020 12:04:18 +0530 Subject: [PATCH v36] Optimize DropRelFileNodeBuffers() for recovery. The recovery path of DropRelFileNodeBuffers() is optimized so that scanning of the whole buffer pool can be avoided when the number of blocks to be truncated in a relation is below a certain threshold. For such cases, we find the buffers by doing lookups in BufMapping table. This improves the performance by more than 100 times in many cases for small tables where the server is configured with a large value of shared buffers (greater than 20GB). This optimization helps cases (a) when vacuum or autovacuum truncated off any of the empty pages at the end of a relation, or (b) when the relation is truncated in the same transaction in which it was created. This commit extends the smgrnblocks interface to return a boolean value which indicates that the cached value for the number of blocks is returned. This helps us to determine the exact size of relation which is required to apply this optimization. The exact size is required to ensure that we don't leave any buffer for the relation being dropped as otherwise the background writer or checkpointer can lead to a PANIC error while flushing buffers corresponding to files that don't exist. Author: Kirk Jamison Reviewed-by: Kyotaro Horiguchi, Takayuki Tsunakawa, and Amit Kapila Discussion: https://postgr.es/m/OSBPR01MB3207DCA7EC725FDD661B3EDAEF660@OSBPR01MB3207.jpnprd01.prod.outlook.com --- src/backend/access/gist/gistbuild.c | 2 +- src/backend/access/heap/visibilitymap.c | 6 +- src/backend/access/table/tableam.c | 4 +- src/backend/access/transam/xlogutils.c | 2 +- src/backend/catalog/storage.c | 4 +- src/backend/storage/buffer/bufmgr.c | 144 +++++++++++++++++++++++++++--- src/backend/storage/freespace/freespace.c | 6 +- src/backend/storage/smgr/smgr.c | 15 +++- src/include/storage/bufmgr.h | 2 +- src/include/storage/smgr.h | 3 +- 10 files changed, 162 insertions(+), 26 deletions(-) diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index 9d3fa9c..d4a3bff 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -860,7 +860,7 @@ gistBuildCallback(Relation index, */ if ((buildstate->buildMode == GIST_BUFFERING_AUTO && buildstate->indtuples % BUFFERING_MODE_SWITCH_CHECK_STEP == 0 && - effective_cache_size < smgrnblocks(index->rd_smgr, MAIN_FORKNUM)) || + effective_cache_size < smgrnblocks(index->rd_smgr, MAIN_FORKNUM, NULL)) || (buildstate->buildMode == GIST_BUFFERING_STATS && buildstate->indtuples >= BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET)) { diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index b107218..ef8533e 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -528,7 +528,7 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks) else newnblocks = truncBlock; - if (smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM) <= newnblocks) + if (smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM, NULL) <= newnblocks) { /* nothing to do, the file was already smaller than requested size */ return InvalidBlockNumber; @@ -564,7 +564,7 @@ vm_readbuf(Relation rel, BlockNumber blkno, bool extend) if (rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) { if (smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM)) - smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM); + smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM, NULL); else rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = 0; } @@ -647,7 +647,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) /* Invalidate cache so that smgrnblocks() asks the kernel. */ rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber; - vm_nblocks_now = smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM); + vm_nblocks_now = smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM, NULL); /* Now extend the file */ while (vm_nblocks_now < vm_nblocks) diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 6438c45..75eade8 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -636,10 +636,10 @@ table_block_relation_size(Relation rel, ForkNumber forkNumber) if (forkNumber == InvalidForkNumber) { for (int i = 0; i < MAX_FORKNUM; i++) - nblocks += smgrnblocks(rel->rd_smgr, i); + nblocks += smgrnblocks(rel->rd_smgr, i, NULL); } else - nblocks = smgrnblocks(rel->rd_smgr, forkNumber); + nblocks = smgrnblocks(rel->rd_smgr, forkNumber, NULL); return nblocks * BLCKSZ; } diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index e0ca385..f4c0e01 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -459,7 +459,7 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum, */ smgrcreate(smgr, forknum, true); - lastblock = smgrnblocks(smgr, forknum); + lastblock = smgrnblocks(smgr, forknum, NULL); if (blkno < lastblock) { diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index d538f257..3874ff3 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -434,7 +434,7 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst, use_wal = XLogIsNeeded() && (relpersistence == RELPERSISTENCE_PERMANENT || copying_initfork); - nblocks = smgrnblocks(src, forkNum); + nblocks = smgrnblocks(src, forkNum, NULL); for (blkno = 0; blkno < nblocks; blkno++) { @@ -721,7 +721,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker) { if (smgrexists(srel, fork)) { - BlockNumber n = smgrnblocks(srel, fork); + BlockNumber n = smgrnblocks(srel, fork, NULL); /* we shouldn't come here for unlogged relations */ Assert(fork != INIT_FORKNUM); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index c5e8707..984a8e1 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -70,6 +70,14 @@ #define RELS_BSEARCH_THRESHOLD 20 +/* + * This is the size (in the number of blocks) above which we scan the + * entire buffer pool to remove the buffers for all the pages of relation + * being dropped. For the relations with size below this threshold, we find + * the buffers by doing lookups in BufMapping table. + */ +#define BUF_DROP_FULL_SCAN_THRESHOLD (uint32) (NBuffers / 256) + typedef struct PrivateRefCountEntry { Buffer buffer; @@ -473,6 +481,10 @@ static BufferDesc *BufferAlloc(SMgrRelation smgr, BufferAccessStrategy strategy, bool *foundPtr); static void FlushBuffer(BufferDesc *buf, SMgrRelation reln); +static void FindAndDropRelFileNodeBuffers(RelFileNode rnode, + ForkNumber forkNum, + BlockNumber nForkBlock, + BlockNumber firstDelBlock); static void AtProcExit_Buffers(int code, Datum arg); static void CheckForBufferLeaks(void); static int rnode_comparator(const void *p1, const void *p2); @@ -740,7 +752,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, /* Substitute proper block number if caller asked for P_NEW */ if (isExtend) - blockNum = smgrnblocks(smgr, forkNum); + blockNum = smgrnblocks(smgr, forkNum, NULL); if (isLocalBuf) { @@ -2856,7 +2868,7 @@ RelationGetNumberOfBlocksInFork(Relation relation, ForkNumber forkNum) /* Open it at the smgr level if not already done */ RelationOpenSmgr(relation); - return smgrnblocks(relation->rd_smgr, forkNum); + return smgrnblocks(relation->rd_smgr, forkNum, NULL); case RELKIND_RELATION: case RELKIND_TOASTVALUE: @@ -2965,19 +2977,20 @@ 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. - * - * XXX currently it sequentially searches the buffer pool, should be - * changed to more clever ways of searching. However, this routine - * is used only in code paths that aren't very performance-critical, - * and we shouldn't slow down the hot paths to make it faster ... * -------------------------------------------------------------------- */ void -DropRelFileNodeBuffers(RelFileNodeBackend rnode, ForkNumber *forkNum, +DropRelFileNodeBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, int nforks, BlockNumber *firstDelBlock) { int i; int j; + RelFileNodeBackend rnode; + bool cached = false; + BlockNumber nForkBlock[MAX_FORKNUM]; + BlockNumber nBlocksToInvalidate = 0; + + rnode = smgr_reln->smgr_rnode; /* If it's a local relation, it's localbuf.c's problem. */ if (RelFileNodeBackendIsTemp(rnode)) @@ -2991,6 +3004,59 @@ DropRelFileNodeBuffers(RelFileNodeBackend rnode, ForkNumber *forkNum, return; } + /* + * To remove all the pages of the specified relation forks from the buffer + * pool, we need to scan the entire buffer pool but we can optimize it by + * finding the buffers from BufMapping table provided we know the exact + * size of each fork of the relation. The exact size is required to ensure + * that we don't leave any buffer for the relation being dropped as + * 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. + * + * 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 + * smgrextend. It is possible that the value of the first lseek is smaller + * than the actual number of existing blocks in the file due to buggy + * Linux kernels that might not have accounted for the recent write. But + * that should be fine because there must not be any buffers after that + * file size. + * + * XXX We would make the extra lseek call for the unoptimized paths but + * that is okay because we do it just for the first fork and we anyway + * have to scan the entire buffer pool the cost of which is so high that + * the extra lseek call won't make any visible difference. However, we can + * use InRecovery flag to avoid the additional cost but that doesn't seem + * worth it. + */ + for (i = 0; i < nforks; i++) + { + /* Get the number of blocks for a relation's fork */ + nForkBlock[i] = smgrnblocks(smgr_reln, forkNum[i], &cached); + + if (!cached) + break; + + /* Get the number of blocks to be invalidated */ + nBlocksToInvalidate += (nForkBlock[i] - firstDelBlock[i]); + } + + /* + * We apply the optimization iff the total number of blocks to invalidate + * is below the BUF_DROP_FULL_SCAN_THRESHOLD. + */ + if (cached && nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD) + { + for (j = 0; j < nforks; j++) + FindAndDropRelFileNodeBuffers(rnode.node, forkNum[j], + nForkBlock[j], firstDelBlock[j]); + return; + } + for (i = 0; i < NBuffers; i++) { BufferDesc *bufHdr = GetBufferDescriptor(i); @@ -3134,6 +3200,65 @@ DropRelFileNodesAllBuffers(RelFileNodeBackend *rnodes, int nnodes) } /* --------------------------------------------------------------------- + * FindAndDropRelFileNodeBuffers + * + * This function performs look up in BufMapping table and removes from the + * buffer pool all the pages of the specified relation fork that has block + * number >= firstDelBlock. (In particular, with firstDelBlock = 0, all + * pages are removed.) + * -------------------------------------------------------------------- + */ +static void +FindAndDropRelFileNodeBuffers(RelFileNode rnode, ForkNumber forkNum, + BlockNumber nForkBlock, + BlockNumber firstDelBlock) +{ + BlockNumber curBlock; + + for (curBlock = firstDelBlock; curBlock < nForkBlock; curBlock++) + { + uint32 bufHash; /* hash value for tag */ + BufferTag bufTag; /* identity of requested block */ + LWLock *bufPartitionLock; /* buffer partition lock for it */ + int buf_id; + BufferDesc *bufHdr; + uint32 buf_state; + + /* create a tag so we can lookup the buffer */ + INIT_BUFFERTAG(bufTag, rnode, forkNum, curBlock); + + /* determine its hash code and partition lock ID */ + bufHash = BufTableHashCode(&bufTag); + bufPartitionLock = BufMappingPartitionLock(bufHash); + + /* Check that it is in the buffer pool. If not, do nothing. */ + LWLockAcquire(bufPartitionLock, LW_SHARED); + buf_id = BufTableLookup(&bufTag, bufHash); + LWLockRelease(bufPartitionLock); + + if (buf_id < 0) + continue; + + bufHdr = GetBufferDescriptor(buf_id); + + /* + * We need to lock the buffer header and recheck if the buffer is + * still associated with the same block because the buffer could be + * evicted by some other backend loading blocks for a different + * relation after we release lock on the BufMapping table. + */ + buf_state = LockBufHdr(bufHdr); + + if (RelFileNodeEquals(bufHdr->tag.rnode, rnode) && + bufHdr->tag.forkNum == forkNum && + bufHdr->tag.blockNum >= firstDelBlock) + InvalidateBuffer(bufHdr); /* releases spinlock */ + else + UnlockBufHdr(bufHdr, buf_state); + } +} + +/* --------------------------------------------------------------------- * DropDatabaseBuffers * * This function removes all the buffers in the buffer cache for a @@ -3245,8 +3370,7 @@ PrintPinnedBufs(void) * XXX currently it sequentially searches the buffer pool, should be * changed to more clever ways of searching. This routine is not * used in any performance-critical code paths, so it's not worth - * adding additional overhead to normal paths to make it go faster; - * but see also DropRelFileNodeBuffers. + * adding additional overhead to normal paths to make it go faster. * -------------------------------------------------------------------- */ void diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c index 6a96126..2ac802c 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -317,7 +317,7 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks) else { new_nfsmblocks = fsm_logical_to_physical(first_removed_address); - if (smgrnblocks(rel->rd_smgr, FSM_FORKNUM) <= new_nfsmblocks) + if (smgrnblocks(rel->rd_smgr, FSM_FORKNUM, NULL) <= new_nfsmblocks) return InvalidBlockNumber; /* nothing to do; the FSM was already * smaller */ } @@ -547,7 +547,7 @@ fsm_readbuf(Relation rel, FSMAddress addr, bool extend) /* Invalidate the cache so smgrnblocks asks the kernel. */ rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber; if (smgrexists(rel->rd_smgr, FSM_FORKNUM)) - smgrnblocks(rel->rd_smgr, FSM_FORKNUM); + smgrnblocks(rel->rd_smgr, FSM_FORKNUM, NULL); else rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = 0; } @@ -633,7 +633,7 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks) /* Invalidate cache so that smgrnblocks() asks the kernel. */ rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber; - fsm_nblocks_now = smgrnblocks(rel->rd_smgr, FSM_FORKNUM); + fsm_nblocks_now = smgrnblocks(rel->rd_smgr, FSM_FORKNUM, NULL); while (fsm_nblocks_now < fsm_nblocks) { diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 072bdd1..360c78e 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -543,9 +543,12 @@ smgrwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, /* * smgrnblocks() -- Calculate the number of blocks in the * supplied relation. + * + * The cached flag indicates that the cached value for the number of blocks + * is returned. We set this only if requested by the caller. */ BlockNumber -smgrnblocks(SMgrRelation reln, ForkNumber forknum) +smgrnblocks(SMgrRelation reln, ForkNumber forknum, bool *cached) { BlockNumber result; @@ -554,7 +557,15 @@ smgrnblocks(SMgrRelation reln, ForkNumber forknum) * invalidation mechanism for changes in file size. */ if (InRecovery && reln->smgr_cached_nblocks[forknum] != InvalidBlockNumber) + { + if (cached) + *cached = true; + return reln->smgr_cached_nblocks[forknum]; + } + + if (cached) + *cached = false; result = smgrsw[reln->smgr_which].smgr_nblocks(reln, forknum); @@ -582,7 +593,7 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *nb * Get rid of any buffers for the about-to-be-deleted blocks. bufmgr will * just drop them without bothering to write the contents. */ - DropRelFileNodeBuffers(reln->smgr_rnode, forknum, nforks, nblocks); + DropRelFileNodeBuffers(reln, forknum, nforks, 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 ee91b8f..056f65e 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -203,7 +203,7 @@ extern void FlushOneBuffer(Buffer buffer); extern void FlushRelationBuffers(Relation rel); extern void FlushRelationsAllBuffers(struct SMgrRelationData **smgrs, int nrels); extern void FlushDatabaseBuffers(Oid dbid); -extern void DropRelFileNodeBuffers(RelFileNodeBackend rnode, ForkNumber *forkNum, +extern void DropRelFileNodeBuffers(struct SMgrRelationData *smgr_reln, ForkNumber *forkNum, int nforks, BlockNumber *firstDelBlock); extern void DropRelFileNodesAllBuffers(RelFileNodeBackend *rnodes, int nnodes); extern void DropDatabaseBuffers(Oid dbid); diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index f28a842..cd99f1b 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -98,7 +98,8 @@ extern void smgrwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char *buffer, bool skipFsync); extern void smgrwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, BlockNumber nblocks); -extern BlockNumber smgrnblocks(SMgrRelation reln, ForkNumber forknum); +extern BlockNumber smgrnblocks(SMgrRelation reln, ForkNumber forknum, + bool *cached); extern void smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *nblocks); extern void smgrimmedsync(SMgrRelation reln, ForkNumber forknum); -- 1.8.3.1