Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9a299cf

Browse files
committed
Replace RelationOpenSmgr() with RelationGetSmgr().
This is a back-patch of the v15-era commitf10f0ae into oldersupported branches. The idea is to design out bugs in which anill-timed relcache flush clears rel->rd_smgr partway throughsome code sequence that wasn't expecting that. We had anotherreport today of a corner case that reliably crashes v14 underdebug_discard_caches (nee CLOBBER_CACHE_ALWAYS), and thereforewould crash once in a blue moon in the field. We're unlikelyto get rid of all such code paths unless we adopt the morerigorous coding rules instituted byf10f0ae. Therefore,even though this is a bit invasive, it's time to back-patch.Some comfort can be taken in the fact thatf10f0ae has beenin v15 for 16 months without problems.I left the RelationOpenSmgr macro present in the back branches,even though no core code should use it anymore, in order to not breakthird-party extensions in minor releases. Such extensions might optto start using RelationGetSmgr instead, to reduce their codedifferential between v15 and earlier branches. This carries a hazardof failing to compile against headers from existing minor releases.However, once compiled the extension should work fine even with suchreleases, because RelationGetSmgr is a "static inline" function soit creates no link-time dependency. So depending on distributionpractices, that might be an OK tradeoff.Per report from Spyridon Dimitrios Agathos. Original patchby Amul Sul.Discussion:https://postgr.es/m/CAFM5RaqdgyusQvmWkyPYaWMwoK5gigdtW-7HcgHgOeAw7mqJ_Q@mail.gmail.comDiscussion:https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
1 parentd419d39 commit9a299cf

File tree

21 files changed

+166
-152
lines changed

21 files changed

+166
-152
lines changed

‎contrib/amcheck/verify_nbtree.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
319319
boolheapkeyspace,
320320
allequalimage;
321321

322-
RelationOpenSmgr(indrel);
323-
if (!smgrexists(indrel->rd_smgr,MAIN_FORKNUM))
322+
if (!smgrexists(RelationGetSmgr(indrel),MAIN_FORKNUM))
324323
ereport(ERROR,
325324
(errcode(ERRCODE_INDEX_CORRUPTED),
326325
errmsg("index \"%s\" lacks a main relation fork",

‎contrib/bloom/blinsert.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,17 +178,17 @@ blbuildempty(Relation index)
178178
* this even when wal_level=minimal.
179179
*/
180180
PageSetChecksumInplace(metapage,BLOOM_METAPAGE_BLKNO);
181-
smgrwrite(index->rd_smgr,INIT_FORKNUM,BLOOM_METAPAGE_BLKNO,
181+
smgrwrite(RelationGetSmgr(index),INIT_FORKNUM,BLOOM_METAPAGE_BLKNO,
182182
(char*)metapage, true);
183-
log_newpage(&index->rd_smgr->smgr_rnode.node,INIT_FORKNUM,
183+
log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node,INIT_FORKNUM,
184184
BLOOM_METAPAGE_BLKNO,metapage, true);
185185

186186
/*
187187
* An immediate sync is required even if we xlog'd the page, because the
188188
* write did not go through shared_buffers and therefore a concurrent
189189
* checkpoint may have moved the redo pointer past our xlog record.
190190
*/
191-
smgrimmedsync(index->rd_smgr,INIT_FORKNUM);
191+
smgrimmedsync(RelationGetSmgr(index),INIT_FORKNUM);
192192
}
193193

194194
/*

‎contrib/pg_prewarm/autoprewarm.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,15 +518,13 @@ autoprewarm_database_main(Datum main_arg)
518518
old_blk->filenode!=blk->filenode||
519519
old_blk->forknum!=blk->forknum)
520520
{
521-
RelationOpenSmgr(rel);
522-
523521
/*
524522
* smgrexists is not safe for illegal forknum, hence check whether
525523
* the passed forknum is valid before using it in smgrexists.
526524
*/
527525
if (blk->forknum>InvalidForkNumber&&
528526
blk->forknum <=MAX_FORKNUM&&
529-
smgrexists(rel->rd_smgr,blk->forknum))
527+
smgrexists(RelationGetSmgr(rel),blk->forknum))
530528
nblocks=RelationGetNumberOfBlocksInFork(rel,blk->forknum);
531529
else
532530
nblocks=0;

‎contrib/pg_prewarm/pg_prewarm.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
109109
aclcheck_error(aclresult,get_relkind_objtype(rel->rd_rel->relkind),get_rel_name(relOid));
110110

111111
/* Check that the fork exists. */
112-
RelationOpenSmgr(rel);
113-
if (!smgrexists(rel->rd_smgr,forkNumber))
112+
if (!smgrexists(RelationGetSmgr(rel),forkNumber))
114113
ereport(ERROR,
115114
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
116115
errmsg("fork \"%s\" does not exist for this relation",
@@ -178,7 +177,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
178177
for (block=first_block;block <=last_block;++block)
179178
{
180179
CHECK_FOR_INTERRUPTS();
181-
smgrread(rel->rd_smgr,forkNumber,block,blockbuffer.data);
180+
smgrread(RelationGetSmgr(rel),forkNumber,block,blockbuffer.data);
182181
++blocks_done;
183182
}
184183
}

‎contrib/pg_visibility/pg_visibility.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,14 +391,14 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS)
391391
/* Only some relkinds have a visibility map */
392392
check_relation_relkind(rel);
393393

394-
RelationOpenSmgr(rel);
395-
rel->rd_smgr->smgr_vm_nblocks=InvalidBlockNumber;
394+
/* Forcibly reset cached file size */
395+
RelationGetSmgr(rel)->smgr_vm_nblocks=InvalidBlockNumber;
396396

397397
block=visibilitymap_prepare_truncate(rel,0);
398398
if (BlockNumberIsValid(block))
399399
{
400400
fork=VISIBILITYMAP_FORKNUM;
401-
smgrtruncate(rel->rd_smgr,&fork,1,&block);
401+
smgrtruncate(RelationGetSmgr(rel),&fork,1,&block);
402402
}
403403

404404
if (RelationNeedsWAL(rel))

‎src/backend/access/gist/gistbuild.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ gistBuildCallback(Relation index,
495495
*/
496496
if ((buildstate->bufferingMode==GIST_BUFFERING_AUTO&&
497497
buildstate->indtuples %BUFFERING_MODE_SWITCH_CHECK_STEP==0&&
498-
effective_cache_size<smgrnblocks(index->rd_smgr,MAIN_FORKNUM))||
498+
effective_cache_size<smgrnblocks(RelationGetSmgr(index),MAIN_FORKNUM))||
499499
(buildstate->bufferingMode==GIST_BUFFERING_STATS&&
500500
buildstate->indtuples >=BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET))
501501
{

‎src/backend/access/hash/hashpage.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,9 +1028,9 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
10281028
zerobuf.data,
10291029
true);
10301030

1031-
RelationOpenSmgr(rel);
10321031
PageSetChecksumInplace(page,lastblock);
1033-
smgrextend(rel->rd_smgr,MAIN_FORKNUM,lastblock,zerobuf.data, false);
1032+
smgrextend(RelationGetSmgr(rel),MAIN_FORKNUM,lastblock,zerobuf.data,
1033+
false);
10341034

10351035
return true;
10361036
}

‎src/backend/access/heap/heapam_handler.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,6 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
626626
SMgrRelationdstrel;
627627

628628
dstrel=smgropen(*newrnode,rel->rd_backend);
629-
RelationOpenSmgr(rel);
630629

631630
/*
632631
* Since we copy the file directly without looking at the shared buffers,
@@ -646,14 +645,14 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
646645
RelationCreateStorage(*newrnode,rel->rd_rel->relpersistence);
647646

648647
/* copy main fork */
649-
RelationCopyStorage(rel->rd_smgr,dstrel,MAIN_FORKNUM,
648+
RelationCopyStorage(RelationGetSmgr(rel),dstrel,MAIN_FORKNUM,
650649
rel->rd_rel->relpersistence);
651650

652651
/* copy those extra forks that exist */
653652
for (ForkNumberforkNum=MAIN_FORKNUM+1;
654653
forkNum <=MAX_FORKNUM;forkNum++)
655654
{
656-
if (smgrexists(rel->rd_smgr,forkNum))
655+
if (smgrexists(RelationGetSmgr(rel),forkNum))
657656
{
658657
smgrcreate(dstrel,forkNum, false);
659658

@@ -665,7 +664,7 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
665664
(rel->rd_rel->relpersistence==RELPERSISTENCE_UNLOGGED&&
666665
forkNum==INIT_FORKNUM))
667666
log_smgrcreate(newrnode,forkNum);
668-
RelationCopyStorage(rel->rd_smgr,dstrel,forkNum,
667+
RelationCopyStorage(RelationGetSmgr(rel),dstrel,forkNum,
669668
rel->rd_rel->relpersistence);
670669
}
671670
}

‎src/backend/access/heap/rewriteheap.c

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,8 @@ end_heap_rewrite(RewriteState state)
327327

328328
PageSetChecksumInplace(state->rs_buffer,state->rs_blockno);
329329

330-
RelationOpenSmgr(state->rs_new_rel);
331-
smgrextend(state->rs_new_rel->rd_smgr,MAIN_FORKNUM,state->rs_blockno,
332-
(char*)state->rs_buffer, true);
330+
smgrextend(RelationGetSmgr(state->rs_new_rel),MAIN_FORKNUM,
331+
state->rs_blockno, (char*)state->rs_buffer, true);
333332
}
334333

335334
/*
@@ -340,11 +339,7 @@ end_heap_rewrite(RewriteState state)
340339
* wrote before the checkpoint.
341340
*/
342341
if (RelationNeedsWAL(state->rs_new_rel))
343-
{
344-
/* for an empty table, this could be first smgr access */
345-
RelationOpenSmgr(state->rs_new_rel);
346-
smgrimmedsync(state->rs_new_rel->rd_smgr,MAIN_FORKNUM);
347-
}
342+
smgrimmedsync(RelationGetSmgr(state->rs_new_rel),MAIN_FORKNUM);
348343

349344
logical_end_heap_rewrite(state);
350345

@@ -692,11 +687,9 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
692687
* need for smgr to schedule an fsync for this write; we'll do it
693688
* ourselves in end_heap_rewrite.
694689
*/
695-
RelationOpenSmgr(state->rs_new_rel);
696-
697690
PageSetChecksumInplace(page,state->rs_blockno);
698691

699-
smgrextend(state->rs_new_rel->rd_smgr,MAIN_FORKNUM,
692+
smgrextend(RelationGetSmgr(state->rs_new_rel),MAIN_FORKNUM,
700693
state->rs_blockno, (char*)page, true);
701694

702695
state->rs_blockno++;

‎src/backend/access/heap/visibilitymap.c

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -455,13 +455,11 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks)
455455
elog(DEBUG1,"vm_truncate %s %d",RelationGetRelationName(rel),nheapblocks);
456456
#endif
457457

458-
RelationOpenSmgr(rel);
459-
460458
/*
461459
* If no visibility map has been created yet for this relation, there's
462460
* nothing to truncate.
463461
*/
464-
if (!smgrexists(rel->rd_smgr,VISIBILITYMAP_FORKNUM))
462+
if (!smgrexists(RelationGetSmgr(rel),VISIBILITYMAP_FORKNUM))
465463
returnInvalidBlockNumber;
466464

467465
/*
@@ -528,7 +526,7 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks)
528526
else
529527
newnblocks=truncBlock;
530528

531-
if (smgrnblocks(rel->rd_smgr,VISIBILITYMAP_FORKNUM) <=newnblocks)
529+
if (smgrnblocks(RelationGetSmgr(rel),VISIBILITYMAP_FORKNUM) <=newnblocks)
532530
{
533531
/* nothing to do, the file was already smaller than requested size */
534532
returnInvalidBlockNumber;
@@ -547,31 +545,30 @@ static Buffer
547545
vm_readbuf(Relationrel,BlockNumberblkno,boolextend)
548546
{
549547
Bufferbuf;
548+
SMgrRelationreln;
550549

551550
/*
552-
* We might not have opened the relation at the smgr level yet, or we
553-
* might have been forced to close it by a sinval message. The code below
554-
* won't necessarily notice relation extension immediately when extend =
555-
* false, so we rely on sinval messages to ensure that our ideas about the
556-
* size of the map aren't too far out of date.
551+
* Caution: re-using this smgr pointer could fail if the relcache entry
552+
* gets closed. It's safe as long as we only do smgr-level operations
553+
* between here and the last use of the pointer.
557554
*/
558-
RelationOpenSmgr(rel);
555+
reln=RelationGetSmgr(rel);
559556

560557
/*
561558
* If we haven't cached the size of the visibility map fork yet, check it
562559
* first.
563560
*/
564-
if (rel->rd_smgr->smgr_vm_nblocks==InvalidBlockNumber)
561+
if (reln->smgr_vm_nblocks==InvalidBlockNumber)
565562
{
566-
if (smgrexists(rel->rd_smgr,VISIBILITYMAP_FORKNUM))
567-
rel->rd_smgr->smgr_vm_nblocks=smgrnblocks(rel->rd_smgr,
568-
VISIBILITYMAP_FORKNUM);
563+
if (smgrexists(reln,VISIBILITYMAP_FORKNUM))
564+
reln->smgr_vm_nblocks=smgrnblocks(reln,
565+
VISIBILITYMAP_FORKNUM);
569566
else
570-
rel->rd_smgr->smgr_vm_nblocks=0;
567+
reln->smgr_vm_nblocks=0;
571568
}
572569

573570
/* Handle requests beyond EOF */
574-
if (blkno >=rel->rd_smgr->smgr_vm_nblocks)
571+
if (blkno >=reln->smgr_vm_nblocks)
575572
{
576573
if (extend)
577574
vm_extend(rel,blkno+1);
@@ -619,6 +616,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
619616
{
620617
BlockNumbervm_nblocks_now;
621618
PGAlignedBlockpg;
619+
SMgrRelationreln;
622620

623621
PageInit((Page)pg.data,BLCKSZ,0);
624622

@@ -634,26 +632,30 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
634632
*/
635633
LockRelationForExtension(rel,ExclusiveLock);
636634

637-
/* Might have to re-open if a cache flush happened */
638-
RelationOpenSmgr(rel);
635+
/*
636+
* Caution: re-using this smgr pointer could fail if the relcache entry
637+
* gets closed. It's safe as long as we only do smgr-level operations
638+
* between here and the last use of the pointer.
639+
*/
640+
reln=RelationGetSmgr(rel);
639641

640642
/*
641643
* Create the file first if it doesn't exist. If smgr_vm_nblocks is
642644
* positive then it must exist, no need for an smgrexists call.
643645
*/
644-
if ((rel->rd_smgr->smgr_vm_nblocks==0||
645-
rel->rd_smgr->smgr_vm_nblocks==InvalidBlockNumber)&&
646-
!smgrexists(rel->rd_smgr,VISIBILITYMAP_FORKNUM))
647-
smgrcreate(rel->rd_smgr,VISIBILITYMAP_FORKNUM, false);
646+
if ((reln->smgr_vm_nblocks==0||
647+
reln->smgr_vm_nblocks==InvalidBlockNumber)&&
648+
!smgrexists(reln,VISIBILITYMAP_FORKNUM))
649+
smgrcreate(reln,VISIBILITYMAP_FORKNUM, false);
648650

649-
vm_nblocks_now=smgrnblocks(rel->rd_smgr,VISIBILITYMAP_FORKNUM);
651+
vm_nblocks_now=smgrnblocks(reln,VISIBILITYMAP_FORKNUM);
650652

651653
/* Now extend the file */
652654
while (vm_nblocks_now<vm_nblocks)
653655
{
654656
PageSetChecksumInplace((Page)pg.data,vm_nblocks_now);
655657

656-
smgrextend(rel->rd_smgr,VISIBILITYMAP_FORKNUM,vm_nblocks_now,
658+
smgrextend(reln,VISIBILITYMAP_FORKNUM,vm_nblocks_now,
657659
pg.data, false);
658660
vm_nblocks_now++;
659661
}
@@ -665,10 +667,10 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
665667
* to keep checking for creation or extension of the file, which happens
666668
* infrequently.
667669
*/
668-
CacheInvalidateSmgr(rel->rd_smgr->smgr_rnode);
670+
CacheInvalidateSmgr(reln->smgr_rnode);
669671

670672
/* Update local cache with the up-to-date size */
671-
rel->rd_smgr->smgr_vm_nblocks=vm_nblocks_now;
673+
reln->smgr_vm_nblocks=vm_nblocks_now;
672674

673675
UnlockRelationForExtension(rel,ExclusiveLock);
674676
}

‎src/backend/access/nbtree/nbtree.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,17 +175,17 @@ btbuildempty(Relation index)
175175
* this even when wal_level=minimal.
176176
*/
177177
PageSetChecksumInplace(metapage,BTREE_METAPAGE);
178-
smgrwrite(index->rd_smgr,INIT_FORKNUM,BTREE_METAPAGE,
178+
smgrwrite(RelationGetSmgr(index),INIT_FORKNUM,BTREE_METAPAGE,
179179
(char*)metapage, true);
180-
log_newpage(&index->rd_smgr->smgr_rnode.node,INIT_FORKNUM,
180+
log_newpage(&RelationGetSmgr(index)->smgr_rnode.node,INIT_FORKNUM,
181181
BTREE_METAPAGE,metapage, true);
182182

183183
/*
184184
* An immediate sync is required even if we xlog'd the page, because the
185185
* write did not go through shared_buffers and therefore a concurrent
186186
* checkpoint may have moved the redo pointer past our xlog record.
187187
*/
188-
smgrimmedsync(index->rd_smgr,INIT_FORKNUM);
188+
smgrimmedsync(RelationGetSmgr(index),INIT_FORKNUM);
189189
}
190190

191191
/*

‎src/backend/access/nbtree/nbtsort.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -638,9 +638,6 @@ _bt_blnewpage(uint32 level)
638638
staticvoid
639639
_bt_blwritepage(BTWriteState*wstate,Pagepage,BlockNumberblkno)
640640
{
641-
/* Ensure rd_smgr is open (could have been closed by relcache flush!) */
642-
RelationOpenSmgr(wstate->index);
643-
644641
/* XLOG stuff */
645642
if (wstate->btws_use_wal)
646643
{
@@ -660,7 +657,7 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
660657
if (!wstate->btws_zeropage)
661658
wstate->btws_zeropage= (Page)palloc0(BLCKSZ);
662659
/* don't set checksum for all-zero page */
663-
smgrextend(wstate->index->rd_smgr,MAIN_FORKNUM,
660+
smgrextend(RelationGetSmgr(wstate->index),MAIN_FORKNUM,
664661
wstate->btws_pages_written++,
665662
(char*)wstate->btws_zeropage,
666663
true);
@@ -675,14 +672,14 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
675672
if (blkno==wstate->btws_pages_written)
676673
{
677674
/* extending the file... */
678-
smgrextend(wstate->index->rd_smgr,MAIN_FORKNUM,blkno,
675+
smgrextend(RelationGetSmgr(wstate->index),MAIN_FORKNUM,blkno,
679676
(char*)page, true);
680677
wstate->btws_pages_written++;
681678
}
682679
else
683680
{
684681
/* overwriting a block we zero-filled before */
685-
smgrwrite(wstate->index->rd_smgr,MAIN_FORKNUM,blkno,
682+
smgrwrite(RelationGetSmgr(wstate->index),MAIN_FORKNUM,blkno,
686683
(char*)page, true);
687684
}
688685

@@ -1428,10 +1425,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
14281425
* still not be on disk when the crash occurs.
14291426
*/
14301427
if (wstate->btws_use_wal)
1431-
{
1432-
RelationOpenSmgr(wstate->index);
1433-
smgrimmedsync(wstate->index->rd_smgr,MAIN_FORKNUM);
1434-
}
1428+
smgrimmedsync(RelationGetSmgr(wstate->index),MAIN_FORKNUM);
14351429
}
14361430

14371431
/*

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp