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

Commite21856f

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 parent36dd007 commite21856f

File tree

21 files changed

+173
-148
lines changed

21 files changed

+173
-148
lines changed

‎contrib/amcheck/verify_nbtree.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
301301
{
302302
boolheapkeyspace;
303303

304-
RelationOpenSmgr(indrel);
305-
if (!smgrexists(indrel->rd_smgr,MAIN_FORKNUM))
304+
if (!smgrexists(RelationGetSmgr(indrel),MAIN_FORKNUM))
306305
ereport(ERROR,
307306
(errcode(ERRCODE_INDEX_CORRUPTED),
308307
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
@@ -179,17 +179,17 @@ blbuildempty(Relation index)
179179
* this even when wal_level=minimal.
180180
*/
181181
PageSetChecksumInplace(metapage,BLOOM_METAPAGE_BLKNO);
182-
smgrwrite(index->rd_smgr,INIT_FORKNUM,BLOOM_METAPAGE_BLKNO,
182+
smgrwrite(RelationGetSmgr(index),INIT_FORKNUM,BLOOM_METAPAGE_BLKNO,
183183
(char*)metapage, true);
184-
log_newpage(&index->rd_smgr->smgr_rnode.node,INIT_FORKNUM,
184+
log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node,INIT_FORKNUM,
185185
BLOOM_METAPAGE_BLKNO,metapage, true);
186186

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

195195
/*

‎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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,8 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS)
389389
/* Only some relkinds have a visibility map */
390390
check_relation_relkind(rel);
391391

392-
RelationOpenSmgr(rel);
393-
rel->rd_smgr->smgr_vm_nblocks=InvalidBlockNumber;
392+
/* Forcibly reset cached file size */
393+
RelationGetSmgr(rel)->smgr_vm_nblocks=InvalidBlockNumber;
394394

395395
visibilitymap_truncate(rel,0);
396396

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ gistBuildCallback(Relation index,
514514
*/
515515
if ((buildstate->bufferingMode==GIST_BUFFERING_AUTO&&
516516
buildstate->indtuples %BUFFERING_MODE_SWITCH_CHECK_STEP==0&&
517-
effective_cache_size<smgrnblocks(index->rd_smgr,MAIN_FORKNUM))||
517+
effective_cache_size<smgrnblocks(RelationGetSmgr(index),MAIN_FORKNUM))||
518518
(buildstate->bufferingMode==GIST_BUFFERING_STATS&&
519519
buildstate->indtuples >=BUFFERING_MODE_TUPLE_SIZE_STATS_TARGET))
520520
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,9 +1031,9 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
10311031
zerobuf.data,
10321032
true);
10331033

1034-
RelationOpenSmgr(rel);
10351034
PageSetChecksumInplace(page,lastblock);
1036-
smgrextend(rel->rd_smgr,MAIN_FORKNUM,lastblock,zerobuf.data, false);
1035+
smgrextend(RelationGetSmgr(rel),MAIN_FORKNUM,lastblock,zerobuf.data,
1036+
false);
10371037

10381038
return true;
10391039
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9098,8 +9098,7 @@ heap_sync(Relation rel)
90989098

90999099
/* main heap */
91009100
FlushRelationBuffers(rel);
9101-
/* FlushRelationBuffers will have opened rd_smgr */
9102-
smgrimmedsync(rel->rd_smgr,MAIN_FORKNUM);
9101+
smgrimmedsync(RelationGetSmgr(rel),MAIN_FORKNUM);
91039102

91049103
/* FSM is not critical, don't bother syncing it */
91059104

@@ -9110,7 +9109,7 @@ heap_sync(Relation rel)
91109109

91119110
toastrel=table_open(rel->rd_rel->reltoastrelid,AccessShareLock);
91129111
FlushRelationBuffers(toastrel);
9113-
smgrimmedsync(toastrel->rd_smgr,MAIN_FORKNUM);
9112+
smgrimmedsync(RelationGetSmgr(toastrel),MAIN_FORKNUM);
91149113
table_close(toastrel,AccessShareLock);
91159114
}
91169115
}

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,6 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
643643
SMgrRelationdstrel;
644644

645645
dstrel=smgropen(*newrnode,rel->rd_backend);
646-
RelationOpenSmgr(rel);
647646

648647
/*
649648
* Since we copy the file directly without looking at the shared buffers,
@@ -663,14 +662,14 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
663662
RelationCreateStorage(*newrnode,rel->rd_rel->relpersistence);
664663

665664
/* copy main fork */
666-
RelationCopyStorage(rel->rd_smgr,dstrel,MAIN_FORKNUM,
665+
RelationCopyStorage(RelationGetSmgr(rel),dstrel,MAIN_FORKNUM,
667666
rel->rd_rel->relpersistence);
668667

669668
/* copy those extra forks that exist */
670669
for (ForkNumberforkNum=MAIN_FORKNUM+1;
671670
forkNum <=MAX_FORKNUM;forkNum++)
672671
{
673-
if (smgrexists(rel->rd_smgr,forkNum))
672+
if (smgrexists(RelationGetSmgr(rel),forkNum))
674673
{
675674
smgrcreate(dstrel,forkNum, false);
676675

@@ -682,7 +681,7 @@ heapam_relation_copy_data(Relation rel, const RelFileNode *newrnode)
682681
(rel->rd_rel->relpersistence==RELPERSISTENCE_UNLOGGED&&
683682
forkNum==INIT_FORKNUM))
684683
log_smgrcreate(newrnode,forkNum);
685-
RelationCopyStorage(rel->rd_smgr,dstrel,forkNum,
684+
RelationCopyStorage(RelationGetSmgr(rel),dstrel,forkNum,
686685
rel->rd_rel->relpersistence);
687686
}
688687
}
@@ -2045,18 +2044,23 @@ static uint64
20452044
heapam_relation_size(Relationrel,ForkNumberforkNumber)
20462045
{
20472046
uint64nblocks=0;
2047+
SMgrRelationreln;
20482048

2049-
/* Open it at the smgr level if not already done */
2050-
RelationOpenSmgr(rel);
2049+
/*
2050+
* Caution: re-using this smgr pointer could fail if the relcache entry
2051+
* gets closed. It's safe as long as we only do smgr-level operations
2052+
* between here and the last use of the pointer.
2053+
*/
2054+
reln=RelationGetSmgr(rel);
20512055

20522056
/* InvalidForkNumber indicates returning the size for all forks */
20532057
if (forkNumber==InvalidForkNumber)
20542058
{
20552059
for (inti=0;i<MAX_FORKNUM;i++)
2056-
nblocks+=smgrnblocks(rel->rd_smgr,i);
2060+
nblocks+=smgrnblocks(reln,i);
20572061
}
20582062
else
2059-
nblocks=smgrnblocks(rel->rd_smgr,forkNumber);
2063+
nblocks=smgrnblocks(reln,forkNumber);
20602064

20612065
returnnblocks*BLCKSZ;
20622066
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,10 @@ end_heap_rewrite(RewriteState state)
336336
state->rs_blockno,
337337
state->rs_buffer,
338338
true);
339-
RelationOpenSmgr(state->rs_new_rel);
340339

341340
PageSetChecksumInplace(state->rs_buffer,state->rs_blockno);
342341

343-
smgrextend(state->rs_new_rel->rd_smgr,MAIN_FORKNUM,state->rs_blockno,
342+
smgrextend(RelationGetSmgr(state->rs_new_rel),MAIN_FORKNUM,state->rs_blockno,
344343
(char*)state->rs_buffer, true);
345344
}
346345

@@ -708,11 +707,9 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
708707
* fsync for this write; we'll do it ourselves in
709708
* end_heap_rewrite.
710709
*/
711-
RelationOpenSmgr(state->rs_new_rel);
712-
713710
PageSetChecksumInplace(page,state->rs_blockno);
714711

715-
smgrextend(state->rs_new_rel->rd_smgr,MAIN_FORKNUM,
712+
smgrextend(RelationGetSmgr(state->rs_new_rel),MAIN_FORKNUM,
716713
state->rs_blockno, (char*)page, true);
717714

718715
state->rs_blockno++;

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

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -452,13 +452,11 @@ visibilitymap_truncate(Relation rel, BlockNumber nheapblocks)
452452
elog(DEBUG1,"vm_truncate %s %d",RelationGetRelationName(rel),nheapblocks);
453453
#endif
454454

455-
RelationOpenSmgr(rel);
456-
457455
/*
458456
* If no visibility map has been created yet for this relation, there's
459457
* nothing to truncate.
460458
*/
461-
if (!smgrexists(rel->rd_smgr,VISIBILITYMAP_FORKNUM))
459+
if (!smgrexists(RelationGetSmgr(rel),VISIBILITYMAP_FORKNUM))
462460
return;
463461

464462
/*
@@ -525,14 +523,14 @@ visibilitymap_truncate(Relation rel, BlockNumber nheapblocks)
525523
else
526524
newnblocks=truncBlock;
527525

528-
if (smgrnblocks(rel->rd_smgr,VISIBILITYMAP_FORKNUM) <=newnblocks)
526+
if (smgrnblocks(RelationGetSmgr(rel),VISIBILITYMAP_FORKNUM) <=newnblocks)
529527
{
530528
/* nothing to do, the file was already smaller than requested size */
531529
return;
532530
}
533531

534532
/* Truncate the unused VM pages, and send smgr inval message */
535-
smgrtruncate(rel->rd_smgr,VISIBILITYMAP_FORKNUM,newnblocks);
533+
smgrtruncate(RelationGetSmgr(rel),VISIBILITYMAP_FORKNUM,newnblocks);
536534

537535
/*
538536
* We might as well update the local smgr_vm_nblocks setting. smgrtruncate
@@ -554,31 +552,30 @@ static Buffer
554552
vm_readbuf(Relationrel,BlockNumberblkno,boolextend)
555553
{
556554
Bufferbuf;
555+
SMgrRelationreln;
557556

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

567564
/*
568565
* If we haven't cached the size of the visibility map fork yet, check it
569566
* first.
570567
*/
571-
if (rel->rd_smgr->smgr_vm_nblocks==InvalidBlockNumber)
568+
if (reln->smgr_vm_nblocks==InvalidBlockNumber)
572569
{
573-
if (smgrexists(rel->rd_smgr,VISIBILITYMAP_FORKNUM))
574-
rel->rd_smgr->smgr_vm_nblocks=smgrnblocks(rel->rd_smgr,
575-
VISIBILITYMAP_FORKNUM);
570+
if (smgrexists(reln,VISIBILITYMAP_FORKNUM))
571+
reln->smgr_vm_nblocks=smgrnblocks(reln,
572+
VISIBILITYMAP_FORKNUM);
576573
else
577-
rel->rd_smgr->smgr_vm_nblocks=0;
574+
reln->smgr_vm_nblocks=0;
578575
}
579576

580577
/* Handle requests beyond EOF */
581-
if (blkno >=rel->rd_smgr->smgr_vm_nblocks)
578+
if (blkno >=reln->smgr_vm_nblocks)
582579
{
583580
if (extend)
584581
vm_extend(rel,blkno+1);
@@ -626,6 +623,7 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
626623
{
627624
BlockNumbervm_nblocks_now;
628625
PGAlignedBlockpg;
626+
SMgrRelationreln;
629627

630628
PageInit((Page)pg.data,BLCKSZ,0);
631629

@@ -641,26 +639,30 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
641639
*/
642640
LockRelationForExtension(rel,ExclusiveLock);
643641

644-
/* Might have to re-open if a cache flush happened */
645-
RelationOpenSmgr(rel);
642+
/*
643+
* Caution: re-using this smgr pointer could fail if the relcache entry
644+
* gets closed. It's safe as long as we only do smgr-level operations
645+
* between here and the last use of the pointer.
646+
*/
647+
reln=RelationGetSmgr(rel);
646648

647649
/*
648650
* Create the file first if it doesn't exist. If smgr_vm_nblocks is
649651
* positive then it must exist, no need for an smgrexists call.
650652
*/
651-
if ((rel->rd_smgr->smgr_vm_nblocks==0||
652-
rel->rd_smgr->smgr_vm_nblocks==InvalidBlockNumber)&&
653-
!smgrexists(rel->rd_smgr,VISIBILITYMAP_FORKNUM))
654-
smgrcreate(rel->rd_smgr,VISIBILITYMAP_FORKNUM, false);
653+
if ((reln->smgr_vm_nblocks==0||
654+
reln->smgr_vm_nblocks==InvalidBlockNumber)&&
655+
!smgrexists(reln,VISIBILITYMAP_FORKNUM))
656+
smgrcreate(reln,VISIBILITYMAP_FORKNUM, false);
655657

656-
vm_nblocks_now=smgrnblocks(rel->rd_smgr,VISIBILITYMAP_FORKNUM);
658+
vm_nblocks_now=smgrnblocks(reln,VISIBILITYMAP_FORKNUM);
657659

658660
/* Now extend the file */
659661
while (vm_nblocks_now<vm_nblocks)
660662
{
661663
PageSetChecksumInplace((Page)pg.data,vm_nblocks_now);
662664

663-
smgrextend(rel->rd_smgr,VISIBILITYMAP_FORKNUM,vm_nblocks_now,
665+
smgrextend(reln,VISIBILITYMAP_FORKNUM,vm_nblocks_now,
664666
pg.data, false);
665667
vm_nblocks_now++;
666668
}
@@ -672,10 +674,10 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
672674
* to keep checking for creation or extension of the file, which happens
673675
* infrequently.
674676
*/
675-
CacheInvalidateSmgr(rel->rd_smgr->smgr_rnode);
677+
CacheInvalidateSmgr(reln->smgr_rnode);
676678

677679
/* Update local cache with the up-to-date size */
678-
rel->rd_smgr->smgr_vm_nblocks=vm_nblocks_now;
680+
reln->smgr_vm_nblocks=vm_nblocks_now;
679681

680682
UnlockRelationForExtension(rel,ExclusiveLock);
681683
}

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

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

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

186186
/*

0 commit comments

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