diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 7f8231a681..f37ca1313d 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -322,8 +322,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed, bool heapkeyspace, allequalimage; - RelationOpenSmgr(indrel); - if (!smgrexists(indrel->rd_smgr, MAIN_FORKNUM)) + if (!smgrexists(RelationGetSmgr(indrel), MAIN_FORKNUM)) ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), errmsg("index \"%s\" lacks a main relation fork", diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c index c34a640d1c..23661d1105 100644 --- a/contrib/bloom/blinsert.c +++ b/contrib/bloom/blinsert.c @@ -177,9 +177,9 @@ blbuildempty(Relation index) * this even when wal_level=minimal. */ PageSetChecksumInplace(metapage, BLOOM_METAPAGE_BLKNO); - smgrwrite(index->rd_smgr, INIT_FORKNUM, BLOOM_METAPAGE_BLKNO, + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, BLOOM_METAPAGE_BLKNO, (char *) metapage, true); - log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM, + log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM, BLOOM_METAPAGE_BLKNO, metapage, true); /* @@ -187,7 +187,7 @@ blbuildempty(Relation index) * write did not go through shared_buffers and therefore a concurrent * checkpoint may have moved the redo pointer past our xlog record. */ - smgrimmedsync(index->rd_smgr, INIT_FORKNUM); + smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM); } /* diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index b3f73ea92d..0289ea657c 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -514,15 +514,13 @@ autoprewarm_database_main(Datum main_arg) old_blk->filenode != blk->filenode || old_blk->forknum != blk->forknum) { - RelationOpenSmgr(rel); - /* * smgrexists is not safe for illegal forknum, hence check whether * the passed forknum is valid before using it in smgrexists. */ if (blk->forknum > InvalidForkNumber && blk->forknum <= MAX_FORKNUM && - smgrexists(rel->rd_smgr, blk->forknum)) + smgrexists(RelationGetSmgr(rel), blk->forknum)) nblocks = RelationGetNumberOfBlocksInFork(rel, blk->forknum); else nblocks = 0; diff --git a/contrib/pg_prewarm/pg_prewarm.c b/contrib/pg_prewarm/pg_prewarm.c index 48d0132a0d..0043823974 100644 --- a/contrib/pg_prewarm/pg_prewarm.c +++ b/contrib/pg_prewarm/pg_prewarm.c @@ -109,8 +109,7 @@ pg_prewarm(PG_FUNCTION_ARGS) aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind), get_rel_name(relOid)); /* Check that the fork exists. */ - RelationOpenSmgr(rel); - if (!smgrexists(rel->rd_smgr, forkNumber)) + if (!smgrexists(RelationGetSmgr(rel), forkNumber)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("fork \"%s\" does not exist for this relation", @@ -178,7 +177,7 @@ pg_prewarm(PG_FUNCTION_ARGS) for (block = first_block; block <= last_block; ++block) { CHECK_FOR_INTERRUPTS(); - smgrread(rel->rd_smgr, forkNumber, block, blockbuffer.data); + smgrread(RelationGetSmgr(rel), forkNumber, block, blockbuffer.data); ++blocks_done; } } diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index dd0c124e62..7ccefdb566 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -391,14 +391,14 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS) /* Only some relkinds have a visibility map */ check_relation_relkind(rel); - RelationOpenSmgr(rel); - rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber; + /* Forcibly reset cached file size */ + RelationGetSmgr(rel)->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber; block = visibilitymap_prepare_truncate(rel, 0); if (BlockNumberIsValid(block)) { fork = VISIBILITYMAP_FORKNUM; - smgrtruncate(rel->rd_smgr, &fork, 1, &block); + smgrtruncate(RelationGetSmgr(rel), &fork, 1, &block); } if (RelationNeedsWAL(rel)) diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index ec28bfe89f..0d2184f730 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -409,8 +409,7 @@ gist_indexsortbuild(GISTBuildState *state) * replaced with the real root page at the end. */ page = palloc0(BLCKSZ); - RelationOpenSmgr(state->indexrel); - smgrextend(state->indexrel->rd_smgr, MAIN_FORKNUM, GIST_ROOT_BLKNO, + smgrextend(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, GIST_ROOT_BLKNO, page, true); state->pages_allocated++; state->pages_written++; @@ -450,10 +449,9 @@ gist_indexsortbuild(GISTBuildState *state) gist_indexsortbuild_flush_ready_pages(state); /* Write out the root */ - RelationOpenSmgr(state->indexrel); PageSetLSN(pagestate->page, GistBuildLSN); PageSetChecksumInplace(pagestate->page, GIST_ROOT_BLKNO); - smgrwrite(state->indexrel->rd_smgr, MAIN_FORKNUM, GIST_ROOT_BLKNO, + smgrwrite(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, GIST_ROOT_BLKNO, pagestate->page, true); if (RelationNeedsWAL(state->indexrel)) log_newpage(&state->indexrel->rd_node, MAIN_FORKNUM, GIST_ROOT_BLKNO, @@ -472,10 +470,7 @@ gist_indexsortbuild(GISTBuildState *state) * still not be on disk when the crash occurs. */ if (RelationNeedsWAL(state->indexrel)) - { - RelationOpenSmgr(state->indexrel); - smgrimmedsync(state->indexrel->rd_smgr, MAIN_FORKNUM); - } + smgrimmedsync(RelationGetSmgr(state->indexrel), MAIN_FORKNUM); } /* @@ -577,8 +572,6 @@ gist_indexsortbuild_flush_ready_pages(GISTBuildState *state) if (state->ready_num_pages == 0) return; - RelationOpenSmgr(state->indexrel); - for (int i = 0; i < state->ready_num_pages; i++) { Page page = state->ready_pages[i]; @@ -590,7 +583,8 @@ gist_indexsortbuild_flush_ready_pages(GISTBuildState *state) PageSetLSN(page, GistBuildLSN); PageSetChecksumInplace(page, blkno); - smgrextend(state->indexrel->rd_smgr, MAIN_FORKNUM, blkno, page, true); + smgrextend(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, blkno, page, + true); state->pages_written++; } @@ -875,7 +869,8 @@ 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(RelationGetSmgr(index), + MAIN_FORKNUM)) || (buildstate->buildMode == GIST_BUFFERING_STATS && buildstate->indtuples >= BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET)) { diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c index ae2f374133..14bde24e4d 100644 --- a/src/backend/access/hash/hashpage.c +++ b/src/backend/access/hash/hashpage.c @@ -1028,9 +1028,9 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks) zerobuf.data, true); - RelationOpenSmgr(rel); PageSetChecksumInplace(page, lastblock); - smgrextend(rel->rd_smgr, MAIN_FORKNUM, lastblock, zerobuf.data, false); + smgrextend(RelationGetSmgr(rel), MAIN_FORKNUM, lastblock, zerobuf.data, + false); return true; } diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 66339392d7..a9d8f8474d 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -628,7 +628,6 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode) SMgrRelation dstrel; dstrel = smgropen(*newrnode, rel->rd_backend); - RelationOpenSmgr(rel); /* * Since we copy the file directly without looking at the shared buffers, @@ -648,14 +647,14 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode) RelationCreateStorage(*newrnode, rel->rd_rel->relpersistence); /* copy main fork */ - RelationCopyStorage(rel->rd_smgr, dstrel, MAIN_FORKNUM, + RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, rel->rd_rel->relpersistence); /* copy those extra forks that exist */ for (ForkNumber forkNum = MAIN_FORKNUM + 1; forkNum <= MAX_FORKNUM; forkNum++) { - if (smgrexists(rel->rd_smgr, forkNum)) + if (smgrexists(RelationGetSmgr(rel), forkNum)) { smgrcreate(dstrel, forkNum, false); @@ -667,7 +666,7 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode) (rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED && forkNum == INIT_FORKNUM)) log_smgrcreate(newrnode, forkNum); - RelationCopyStorage(rel->rd_smgr, dstrel, forkNum, + RelationCopyStorage(RelationGetSmgr(rel), dstrel, forkNum, rel->rd_rel->relpersistence); } } diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 15bef9f7d3..2c159cf76a 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -326,9 +326,8 @@ end_heap_rewrite(RewriteState state) PageSetChecksumInplace(state->rs_buffer, state->rs_blockno); - RelationOpenSmgr(state->rs_new_rel); - smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, state->rs_blockno, - (char *) state->rs_buffer, true); + smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM, + state->rs_blockno, (char *) state->rs_buffer, true); } /* @@ -339,11 +338,7 @@ end_heap_rewrite(RewriteState state) * wrote before the checkpoint. */ if (RelationNeedsWAL(state->rs_new_rel)) - { - /* for an empty table, this could be first smgr access */ - RelationOpenSmgr(state->rs_new_rel); - smgrimmedsync(state->rs_new_rel->rd_smgr, MAIN_FORKNUM); - } + smgrimmedsync(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM); logical_end_heap_rewrite(state); @@ -695,11 +690,9 @@ raw_heap_insert(RewriteState state, HeapTuple tup) * need for smgr to schedule an fsync for this write; we'll do it * ourselves in end_heap_rewrite. */ - RelationOpenSmgr(state->rs_new_rel); - PageSetChecksumInplace(page, state->rs_blockno); - smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, + smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM, state->rs_blockno, (char *) page, true); state->rs_blockno++; diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index e198df65d8..4720b35ee5 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -455,13 +455,11 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks) elog(DEBUG1, "vm_truncate %s %d", RelationGetRelationName(rel), nheapblocks); #endif - RelationOpenSmgr(rel); - /* * If no visibility map has been created yet for this relation, there's * nothing to truncate. */ - if (!smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM)) + if (!smgrexists(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM)) return InvalidBlockNumber; /* @@ -528,7 +526,7 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks) else newnblocks = truncBlock; - if (smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM) <= newnblocks) + if (smgrnblocks(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM) <= newnblocks) { /* nothing to do, the file was already smaller than requested size */ return InvalidBlockNumber; @@ -547,30 +545,29 @@ static Buffer vm_readbuf(Relation rel, BlockNumber blkno, bool extend) { Buffer buf; + SMgrRelation reln; /* - * We might not have opened the relation at the smgr level yet, or we - * might have been forced to close it by a sinval message. The code below - * won't necessarily notice relation extension immediately when extend = - * false, so we rely on sinval messages to ensure that our ideas about the - * size of the map aren't too far out of date. + * Caution: re-using this smgr pointer could fail if the relcache entry + * gets closed. It's safe as long as we only do smgr-level operations + * between here and the last use of the pointer. */ - RelationOpenSmgr(rel); + reln = RelationGetSmgr(rel); /* * If we haven't cached the size of the visibility map fork yet, check it * first. */ - if (rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) + if (reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) { - if (smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM)) - smgrnblocks(rel->rd_smgr, VISIBILITYMAP_FORKNUM); + if (smgrexists(reln, VISIBILITYMAP_FORKNUM)) + smgrnblocks(reln, VISIBILITYMAP_FORKNUM); else - rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = 0; + reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = 0; } /* Handle requests beyond EOF */ - if (blkno >= rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM]) + if (blkno >= reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM]) { if (extend) vm_extend(rel, blkno + 1); @@ -618,6 +615,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) { BlockNumber vm_nblocks_now; PGAlignedBlock pg; + SMgrRelation reln; PageInit((Page) pg.data, BLCKSZ, 0); @@ -633,29 +631,32 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) */ LockRelationForExtension(rel, ExclusiveLock); - /* Might have to re-open if a cache flush happened */ - RelationOpenSmgr(rel); + /* + * Caution: re-using this smgr pointer could fail if the relcache entry + * gets closed. It's safe as long as we only do smgr-level operations + * between here and the last use of the pointer. + */ + reln = RelationGetSmgr(rel); /* * Create the file first if it doesn't exist. If smgr_vm_nblocks is * positive then it must exist, no need for an smgrexists call. */ - if ((rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == 0 || - rel->rd_smgr->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) && - !smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM)) - smgrcreate(rel->rd_smgr, VISIBILITYMAP_FORKNUM, false); + if ((reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == 0 || + reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] == InvalidBlockNumber) && + !smgrexists(reln, VISIBILITYMAP_FORKNUM)) + smgrcreate(reln, VISIBILITYMAP_FORKNUM, false); /* 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); + reln->smgr_cached_nblocks[VISIBILITYMAP_FORKNUM] = InvalidBlockNumber; + vm_nblocks_now = smgrnblocks(reln, VISIBILITYMAP_FORKNUM); /* Now extend the file */ while (vm_nblocks_now < vm_nblocks) { PageSetChecksumInplace((Page) pg.data, vm_nblocks_now); - smgrextend(rel->rd_smgr, VISIBILITYMAP_FORKNUM, vm_nblocks_now, - pg.data, false); + smgrextend(reln, VISIBILITYMAP_FORKNUM, vm_nblocks_now, pg.data, false); vm_nblocks_now++; } @@ -666,7 +667,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) * to keep checking for creation or extension of the file, which happens * infrequently. */ - CacheInvalidateSmgr(rel->rd_smgr->smgr_rnode); + CacheInvalidateSmgr(reln->smgr_rnode); UnlockRelationForExtension(rel, ExclusiveLock); } diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 1360ab80c1..be23395dfb 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -163,9 +163,9 @@ btbuildempty(Relation index) * this even when wal_level=minimal. */ PageSetChecksumInplace(metapage, BTREE_METAPAGE); - smgrwrite(index->rd_smgr, INIT_FORKNUM, BTREE_METAPAGE, + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, BTREE_METAPAGE, (char *) metapage, true); - log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM, + log_newpage(&RelationGetSmgr(index)->smgr_rnode.node, INIT_FORKNUM, BTREE_METAPAGE, metapage, true); /* @@ -173,7 +173,7 @@ btbuildempty(Relation index) * write did not go through shared_buffers and therefore a concurrent * checkpoint may have moved the redo pointer past our xlog record. */ - smgrimmedsync(index->rd_smgr, INIT_FORKNUM); + smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM); } /* diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index 78f78e7341..1e02be9746 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -637,9 +637,6 @@ _bt_blnewpage(uint32 level) static void _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno) { - /* Ensure rd_smgr is open (could have been closed by relcache flush!) */ - RelationOpenSmgr(wstate->index); - /* XLOG stuff */ if (wstate->btws_use_wal) { @@ -659,7 +656,7 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno) if (!wstate->btws_zeropage) wstate->btws_zeropage = (Page) palloc0(BLCKSZ); /* don't set checksum for all-zero page */ - smgrextend(wstate->index->rd_smgr, MAIN_FORKNUM, + smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM, wstate->btws_pages_written++, (char *) wstate->btws_zeropage, true); @@ -674,14 +671,14 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno) if (blkno == wstate->btws_pages_written) { /* extending the file... */ - smgrextend(wstate->index->rd_smgr, MAIN_FORKNUM, blkno, + smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM, blkno, (char *) page, true); wstate->btws_pages_written++; } else { /* overwriting a block we zero-filled before */ - smgrwrite(wstate->index->rd_smgr, MAIN_FORKNUM, blkno, + smgrwrite(RelationGetSmgr(wstate->index), MAIN_FORKNUM, blkno, (char *) page, true); } @@ -1428,10 +1425,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2) * still not be on disk when the crash occurs. */ if (wstate->btws_use_wal) - { - RelationOpenSmgr(wstate->index); - smgrimmedsync(wstate->index->rd_smgr, MAIN_FORKNUM); - } + smgrimmedsync(RelationGetSmgr(wstate->index), MAIN_FORKNUM); } /* diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c index 1af0af7da2..cc4394b1c8 100644 --- a/src/backend/access/spgist/spginsert.c +++ b/src/backend/access/spgist/spginsert.c @@ -169,27 +169,27 @@ spgbuildempty(Relation index) * replayed. */ PageSetChecksumInplace(page, SPGIST_METAPAGE_BLKNO); - smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_METAPAGE_BLKNO, + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_METAPAGE_BLKNO, (char *) page, true); - log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM, + log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM, SPGIST_METAPAGE_BLKNO, page, true); /* Likewise for the root page. */ SpGistInitPage(page, SPGIST_LEAF); PageSetChecksumInplace(page, SPGIST_ROOT_BLKNO); - smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_ROOT_BLKNO, + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_ROOT_BLKNO, (char *) page, true); - log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM, + log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM, SPGIST_ROOT_BLKNO, page, true); /* Likewise for the null-tuples root page. */ SpGistInitPage(page, SPGIST_LEAF | SPGIST_NULLS); PageSetChecksumInplace(page, SPGIST_NULL_BLKNO); - smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_NULL_BLKNO, + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_NULL_BLKNO, (char *) page, true); - log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM, + log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM, SPGIST_NULL_BLKNO, page, true); /* @@ -197,7 +197,7 @@ spgbuildempty(Relation index) * writes did not go through shared buffers and therefore a concurrent * checkpoint may have moved the redo pointer past our xlog record. */ - smgrimmedsync(index->rd_smgr, INIT_FORKNUM); + smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM); } /* diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 5ea5bdd810..66f0f84386 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -629,17 +629,14 @@ table_block_relation_size(Relation rel, ForkNumber forkNumber) { uint64 nblocks = 0; - /* Open it at the smgr level if not already done */ - RelationOpenSmgr(rel); - /* InvalidForkNumber indicates returning the size for all forks */ if (forkNumber == InvalidForkNumber) { for (int i = 0; i < MAX_FORKNUM; i++) - nblocks += smgrnblocks(rel->rd_smgr, i); + nblocks += smgrnblocks(RelationGetSmgr(rel), i); } else - nblocks = smgrnblocks(rel->rd_smgr, forkNumber); + nblocks = smgrnblocks(RelationGetSmgr(rel), forkNumber); return nblocks * BLCKSZ; } diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index cc08af647e..3fd44eeb54 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -415,8 +415,6 @@ heap_create(const char *relname, */ if (create_storage) { - RelationOpenSmgr(rel); - switch (rel->rd_rel->relkind) { case RELKIND_VIEW: diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index a90cc3a8a0..ad6a4f490b 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3021,10 +3021,9 @@ index_build(Relation heapRelation, * relfilenode won't change, and nothing needs to be done here. */ if (indexRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED && - !smgrexists(indexRelation->rd_smgr, INIT_FORKNUM)) + !smgrexists(RelationGetSmgr(indexRelation), INIT_FORKNUM)) { - RelationOpenSmgr(indexRelation); - smgrcreate(indexRelation->rd_smgr, INIT_FORKNUM, false); + smgrcreate(RelationGetSmgr(indexRelation), INIT_FORKNUM, false); indexRelation->rd_indam->ambuildempty(indexRelation); } diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index b7fc491a68..8f74a3efde 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -282,16 +282,16 @@ RelationTruncate(Relation rel, BlockNumber nblocks) ForkNumber forks[MAX_FORKNUM]; BlockNumber blocks[MAX_FORKNUM]; int nforks = 0; - - /* Open it at the smgr level if not already done */ - RelationOpenSmgr(rel); + SMgrRelation reln; /* - * Make sure smgr_targblock etc aren't pointing somewhere past new end + * Make sure smgr_targblock etc aren't pointing somewhere past new end. + * (Note: don't rely on this reln pointer below this loop.) */ - rel->rd_smgr->smgr_targblock = InvalidBlockNumber; + reln = RelationGetSmgr(rel); + reln->smgr_targblock = InvalidBlockNumber; for (int i = 0; i <= MAX_FORKNUM; ++i) - rel->rd_smgr->smgr_cached_nblocks[i] = InvalidBlockNumber; + reln->smgr_cached_nblocks[i] = InvalidBlockNumber; /* Prepare for truncation of MAIN fork of the relation */ forks[nforks] = MAIN_FORKNUM; @@ -299,7 +299,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks) nforks++; /* Prepare for truncation of the FSM if it exists */ - fsm = smgrexists(rel->rd_smgr, FSM_FORKNUM); + fsm = smgrexists(RelationGetSmgr(rel), FSM_FORKNUM); if (fsm) { blocks[nforks] = FreeSpaceMapPrepareTruncateRel(rel, nblocks); @@ -312,7 +312,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks) } /* Prepare for truncation of the visibility map too if it exists */ - vm = smgrexists(rel->rd_smgr, VISIBILITYMAP_FORKNUM); + vm = smgrexists(RelationGetSmgr(rel), VISIBILITYMAP_FORKNUM); if (vm) { blocks[nforks] = visibilitymap_prepare_truncate(rel, nblocks); @@ -384,7 +384,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks) * longer exist after truncation is complete, and then truncate the * corresponding files on disk. */ - smgrtruncate(rel->rd_smgr, forks, nforks, blocks); + smgrtruncate(RelationGetSmgr(rel), forks, nforks, blocks); /* We've done all the critical work, so checkpoints are OK now. */ MyProc->delayChkptEnd = false; @@ -416,9 +416,9 @@ RelationPreTruncate(Relation rel) if (!pendingSyncHash) return; - RelationOpenSmgr(rel); - pending = hash_search(pendingSyncHash, &(rel->rd_smgr->smgr_rnode.node), + pending = hash_search(pendingSyncHash, + &(RelationGetSmgr(rel)->smgr_rnode.node), HASH_FIND, NULL); if (pending) pending->is_truncated = true; @@ -430,6 +430,12 @@ RelationPreTruncate(Relation rel) * Note that this requires that there is no dirty data in shared buffers. If * it's possible that there are, callers need to flush those using * e.g. FlushRelationBuffers(rel). + * + * Also note that this is frequently called via locutions such as + * RelationCopyStorage(RelationGetSmgr(rel), ...); + * That's safe only because we perform only smgr and WAL operations here. + * If we invoked anything else, a relcache flush could cause our SMgrRelation + * argument to become a dangling pointer. */ void RelationCopyStorage(SMgrRelation src, SMgrRelation dst, @@ -472,13 +478,23 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst, if (!PageIsVerifiedExtended(page, blkno, PIV_LOG_WARNING | PIV_REPORT_STAT)) + { + /* + * For paranoia's sake, capture the file path before invoking the + * ereport machinery. This guards against the possibility of a + * relcache flush caused by, e.g., an errcontext callback. + * (errcontext callbacks shouldn't be risking any such thing, but + * people have been known to forget that rule.) + */ + char *relpath = relpathbackend(src->smgr_rnode.node, + src->smgr_rnode.backend, + forkNum); + ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED), errmsg("invalid page in block %u of relation %s", - blkno, - relpathbackend(src->smgr_rnode.node, - src->smgr_rnode.backend, - forkNum)))); + blkno, relpath))); + } /* * WAL-log the copied page. Unfortunately we don't know what kind of a diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 89fc297fe3..38abc3a76e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14174,7 +14174,6 @@ index_copy_data(Relation rel, RelFileNode newrnode) SMgrRelation dstrel; dstrel = smgropen(newrnode, rel->rd_backend); - RelationOpenSmgr(rel); /* * Since we copy the file directly without looking at the shared buffers, @@ -14194,14 +14193,14 @@ index_copy_data(Relation rel, RelFileNode newrnode) RelationCreateStorage(newrnode, rel->rd_rel->relpersistence); /* copy main fork */ - RelationCopyStorage(rel->rd_smgr, dstrel, MAIN_FORKNUM, + RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, rel->rd_rel->relpersistence); /* copy those extra forks that exist */ for (ForkNumber forkNum = MAIN_FORKNUM + 1; forkNum <= MAX_FORKNUM; forkNum++) { - if (smgrexists(rel->rd_smgr, forkNum)) + if (smgrexists(RelationGetSmgr(rel), forkNum)) { smgrcreate(dstrel, forkNum, false); @@ -14213,7 +14212,7 @@ index_copy_data(Relation rel, RelFileNode newrnode) (rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED && forkNum == INIT_FORKNUM)) log_smgrcreate(&newrnode, forkNum); - RelationCopyStorage(rel->rd_smgr, dstrel, forkNum, + RelationCopyStorage(RelationGetSmgr(rel), dstrel, forkNum, rel->rd_rel->relpersistence); } } diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index d2eb69b5b0..adb39dada4 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -589,9 +589,6 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) Assert(RelationIsValid(reln)); Assert(BlockNumberIsValid(blockNum)); - /* Open it at the smgr level if not already done */ - RelationOpenSmgr(reln); - if (RelationUsesLocalBuffers(reln)) { /* see comments in ReadBufferExtended */ @@ -601,12 +598,12 @@ PrefetchBuffer(Relation reln, ForkNumber forkNum, BlockNumber blockNum) errmsg("cannot access temporary tables of other sessions"))); /* pass it off to localbuf.c */ - return PrefetchLocalBuffer(reln->rd_smgr, forkNum, blockNum); + return PrefetchLocalBuffer(RelationGetSmgr(reln), forkNum, blockNum); } else { /* pass it to the shared buffer version */ - return PrefetchSharedBuffer(reln->rd_smgr, forkNum, blockNum); + return PrefetchSharedBuffer(RelationGetSmgr(reln), forkNum, blockNum); } } @@ -757,9 +754,6 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, bool hit; Buffer buf; - /* Open it at the smgr level if not already done */ - RelationOpenSmgr(reln); - /* * Reject attempts to read non-local temporary relations; we would be * likely to get wrong data since we have no visibility into the owning @@ -775,7 +769,7 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, * miss. */ pgstat_count_buffer_read(reln); - buf = ReadBuffer_common(reln->rd_smgr, reln->rd_rel->relpersistence, + buf = ReadBuffer_common(RelationGetSmgr(reln), reln->rd_rel->relpersistence, forkNum, blockNum, mode, strategy, &hit); if (hit) pgstat_count_buffer_hit(reln); @@ -2968,10 +2962,7 @@ RelationGetNumberOfBlocksInFork(Relation relation, ForkNumber forkNum) case RELKIND_SEQUENCE: case RELKIND_INDEX: case RELKIND_PARTITIONED_INDEX: - /* Open it at the smgr level if not already done */ - RelationOpenSmgr(relation); - - return smgrnblocks(relation->rd_smgr, forkNum); + return smgrnblocks(RelationGetSmgr(relation), forkNum); case RELKIND_RELATION: case RELKIND_TOASTVALUE: @@ -3546,9 +3537,6 @@ FlushRelationBuffers(Relation rel) int i; BufferDesc *bufHdr; - /* Open rel at the smgr level if not already done */ - RelationOpenSmgr(rel); - if (RelationUsesLocalBuffers(rel)) { for (i = 0; i < NLocBuffer; i++) @@ -3573,7 +3561,7 @@ FlushRelationBuffers(Relation rel) PageSetChecksumInplace(localpage, bufHdr->tag.blockNum); - smgrwrite(rel->rd_smgr, + smgrwrite(RelationGetSmgr(rel), bufHdr->tag.forkNum, bufHdr->tag.blockNum, localpage, @@ -3614,7 +3602,7 @@ FlushRelationBuffers(Relation rel) { PinBuffer_Locked(bufHdr); LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED); - FlushBuffer(bufHdr, rel->rd_smgr); + FlushBuffer(bufHdr, RelationGetSmgr(rel)); LWLockRelease(BufferDescriptorGetContentLock(bufHdr)); UnpinBuffer(bufHdr, true); } diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c index 8c12dda238..09d4b16067 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -265,13 +265,11 @@ FreeSpaceMapPrepareTruncateRel(Relation rel, BlockNumber nblocks) uint16 first_removed_slot; Buffer buf; - RelationOpenSmgr(rel); - /* * If no FSM has been created yet for this relation, there's nothing to * truncate. */ - if (!smgrexists(rel->rd_smgr, FSM_FORKNUM)) + if (!smgrexists(RelationGetSmgr(rel), FSM_FORKNUM)) return InvalidBlockNumber; /* Get the location in the FSM of the first removed heap block */ @@ -317,7 +315,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(RelationGetSmgr(rel), FSM_FORKNUM) <= new_nfsmblocks) return InvalidBlockNumber; /* nothing to do; the FSM was already * smaller */ } @@ -532,8 +530,14 @@ fsm_readbuf(Relation rel, FSMAddress addr, bool extend) { BlockNumber blkno = fsm_logical_to_physical(addr); Buffer buf; + SMgrRelation reln; - RelationOpenSmgr(rel); + /* + * Caution: re-using this smgr pointer could fail if the relcache entry + * gets closed. It's safe as long as we only do smgr-level operations + * between here and the last use of the pointer. + */ + reln = RelationGetSmgr(rel); /* * If we haven't cached the size of the FSM yet, check it first. Also @@ -541,19 +545,19 @@ fsm_readbuf(Relation rel, FSMAddress addr, bool extend) * value might be stale. (We send smgr inval messages on truncation, but * not on extension.) */ - if (rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber || - blkno >= rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM]) + if (reln->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber || + blkno >= reln->smgr_cached_nblocks[FSM_FORKNUM]) { /* 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); + reln->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber; + if (smgrexists(reln, FSM_FORKNUM)) + smgrnblocks(reln, FSM_FORKNUM); else - rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] = 0; + reln->smgr_cached_nblocks[FSM_FORKNUM] = 0; } /* Handle requests beyond EOF */ - if (blkno >= rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM]) + if (blkno >= reln->smgr_cached_nblocks[FSM_FORKNUM]) { if (extend) fsm_extend(rel, blkno + 1); @@ -603,6 +607,7 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks) { BlockNumber fsm_nblocks_now; PGAlignedBlock pg; + SMgrRelation reln; PageInit((Page) pg.data, BLCKSZ, 0); @@ -618,28 +623,33 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks) */ LockRelationForExtension(rel, ExclusiveLock); - /* Might have to re-open if a cache flush happened */ - RelationOpenSmgr(rel); + /* + * Caution: re-using this smgr pointer could fail if the relcache entry + * gets closed. It's safe as long as we only do smgr-level operations + * between here and the last use of the pointer. + */ + reln = RelationGetSmgr(rel); /* * Create the FSM file first if it doesn't exist. If * smgr_cached_nblocks[FSM_FORKNUM] is positive then it must exist, no * need for an smgrexists call. */ - if ((rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] == 0 || - rel->rd_smgr->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber) && - !smgrexists(rel->rd_smgr, FSM_FORKNUM)) - smgrcreate(rel->rd_smgr, FSM_FORKNUM, false); + if ((reln->smgr_cached_nblocks[FSM_FORKNUM] == 0 || + reln->smgr_cached_nblocks[FSM_FORKNUM] == InvalidBlockNumber) && + !smgrexists(reln, FSM_FORKNUM)) + smgrcreate(reln, FSM_FORKNUM, false); /* 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); + reln->smgr_cached_nblocks[FSM_FORKNUM] = InvalidBlockNumber; + fsm_nblocks_now = smgrnblocks(reln, FSM_FORKNUM); + /* Extend as needed. */ while (fsm_nblocks_now < fsm_nblocks) { PageSetChecksumInplace((Page) pg.data, fsm_nblocks_now); - smgrextend(rel->rd_smgr, FSM_FORKNUM, fsm_nblocks_now, + smgrextend(reln, FSM_FORKNUM, fsm_nblocks_now, pg.data, false); fsm_nblocks_now++; } diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index d62ac86c52..49920da509 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -24,6 +24,7 @@ #include "rewrite/prs2lock.h" #include "storage/block.h" #include "storage/relfilenode.h" +#include "storage/smgr.h" #include "utils/relcache.h" #include "utils/reltrigger.h" @@ -53,8 +54,7 @@ typedef LockInfoData *LockInfo; typedef struct RelationData { RelFileNode rd_node; /* relation physical identifier */ - /* use "struct" here to avoid needing to include smgr.h: */ - struct SMgrRelationData *rd_smgr; /* cached file handle, or NULL */ + SMgrRelation rd_smgr; /* cached file handle, or NULL */ int rd_refcnt; /* reference count */ BackendId rd_backend; /* owning backend id, if temporary relation */ bool rd_islocaltemp; /* rel is a temp rel of this session */ @@ -526,9 +526,33 @@ typedef struct ViewOptions (RELKIND_HAS_STORAGE((relation)->rd_rel->relkind) && \ ((relation)->rd_rel->relfilenode == InvalidOid)) +/* + * RelationGetSmgr + * Returns smgr file handle for a relation, opening it if needed. + * + * Very little code is authorized to touch rel->rd_smgr directly. Instead + * use this function to fetch its value. + * + * Note: since a relcache flush can cause the file handle to be closed again, + * it's unwise to hold onto the pointer returned by this function for any + * long period. Recommended practice is to just re-execute RelationGetSmgr + * each time you need to access the SMgrRelation. It's quite cheap in + * comparison to whatever an smgr function is going to do. + */ +static inline SMgrRelation +RelationGetSmgr(Relation rel) +{ + if (unlikely(rel->rd_smgr == NULL)) + smgrsetowner(&(rel->rd_smgr), smgropen(rel->rd_node, rel->rd_backend)); + return rel->rd_smgr; +} + /* * RelationOpenSmgr * Open the relation at the smgr level, if not already done. + * + * XXX this is now deprecated, and should not be used in new code. + * Instead, call RelationGetSmgr in place of fetching rd_smgr directly. */ #define RelationOpenSmgr(relation) \ do { \ @@ -556,7 +580,8 @@ typedef struct ViewOptions * Fetch relation's current insertion target block. * * Returns InvalidBlockNumber if there is no current target block. Note - * that the target block status is discarded on any smgr-level invalidation. + * that the target block status is discarded on any smgr-level invalidation, + * so there's no need to re-open the smgr handle if it's not currently open. */ #define RelationGetTargetBlock(relation) \ ( (relation)->rd_smgr != NULL ? (relation)->rd_smgr->smgr_targblock : InvalidBlockNumber ) @@ -567,8 +592,7 @@ typedef struct ViewOptions */ #define RelationSetTargetBlock(relation, targblock) \ do { \ - RelationOpenSmgr(relation); \ - (relation)->rd_smgr->smgr_targblock = (targblock); \ + RelationGetSmgr(relation)->smgr_targblock = (targblock); \ } while (0) /*