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

Commit21d9c3e

Browse files
committed
Give SMgrRelation pointers a well-defined lifetime.
After calling smgropen(), it was not clear how long you could continueto use the result, because various code paths including cacheinvalidation could call smgrclose(), which freed the memory.Guarantee that the object won't be destroyed until the end of thecurrent transaction, or in recovery, the commit/abort record thatdestroys the underlying storage.smgrclose() is now just an alias for smgrrelease(). It closes filesand forgets all state except the rlocator, but keeps the SMgrRelationobject valid.A new smgrdestroy() function is used by rare places that know thereshould be no other references to the SMgrRelation.The short version: * smgrclose() is now just an alias for smgrrelease(). It releases resources, but doesn't destroy until EOX * smgrdestroy() now frees memory, and should rarely be used.Existing code should be unaffected, but it is now possible for code thathas an SMgrRelation object to use it repeatedly during a transaction aslong as the storage hasn't been physically dropped. Such code wouldnormally hold a lock on the relation.This also replaces the "ownership" mechanism of SMgrRelations with apin counter. An SMgrRelation can now be "pinned", which prevents itfrom being destroyed at end of transaction. There can be multiple pinson the same SMgrRelation. In practice, the pin mechanism is only usedby the relcache, so there cannot be more than one pin on the sameSMgrRelation. Except with swap_relation_files XXXAuthor: Thomas Munro, Heikki LinnakangasReviewed-by: Robert Haas <robertmhaas@gmail.com>Discussion:https://www.postgresql.org/message-id/CA%2BhUKGJ8NTvqLHz6dqbQnt2c8XCki4r2QvXjBQcXpVwxTY_pvA@mail.gmail.com
1 parent6a8ffe8 commit21d9c3e

File tree

12 files changed

+183
-227
lines changed

12 files changed

+183
-227
lines changed

‎src/backend/access/transam/xlogutils.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,11 @@ CreateFakeRelcacheEntry(RelFileLocator rlocator)
616616
rel->rd_lockInfo.lockRelId.dbId=rlocator.dbOid;
617617
rel->rd_lockInfo.lockRelId.relId=rlocator.relNumber;
618618

619-
rel->rd_smgr=NULL;
619+
/*
620+
* Set up a non-pinned SMgrRelation reference, so that we don't need to
621+
* worry about unpinning it on error.
622+
*/
623+
rel->rd_smgr=smgropen(rlocator,InvalidBackendId);
620624

621625
returnrel;
622626
}
@@ -627,9 +631,6 @@ CreateFakeRelcacheEntry(RelFileLocator rlocator)
627631
void
628632
FreeFakeRelcacheEntry(Relationfakerel)
629633
{
630-
/* make sure the fakerel is not referenced by the SmgrRelation anymore */
631-
if (fakerel->rd_smgr!=NULL)
632-
smgrclearowner(&fakerel->rd_smgr,fakerel->rd_smgr);
633634
pfree(fakerel);
634635
}
635636

@@ -656,10 +657,10 @@ XLogDropDatabase(Oid dbid)
656657
/*
657658
* This is unnecessarily heavy-handed, as it will close SMgrRelation
658659
* objects for other databases as well. DROP DATABASE occurs seldom enough
659-
* that it's not worth introducing a variant ofsmgrclose for just this
660-
* purpose. XXX: Or should we rather leave the smgr entries dangling?
660+
* that it's not worth introducing a variant ofsmgrdestroy for just this
661+
* purpose.
661662
*/
662-
smgrcloseall();
663+
smgrdestroyall();
663664

664665
forget_invalid_pages_db(dbid);
665666
}

‎src/backend/commands/cluster.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,25 +1422,6 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
14221422
heap_freetuple(reltup2);
14231423

14241424
table_close(relRelation,RowExclusiveLock);
1425-
1426-
/*
1427-
* Close both relcache entries' smgr links. We need this kluge because
1428-
* both links will be invalidated during upcoming CommandCounterIncrement.
1429-
* Whichever of the rels is the second to be cleared will have a dangling
1430-
* reference to the other's smgr entry. Rather than trying to avoid this
1431-
* by ordering operations just so, it's easiest to close the links first.
1432-
* (Fortunately, since one of the entries is local in our transaction,
1433-
* it's sufficient to clear out our own relcache this way; the problem
1434-
* cannot arise for other backends when they see our update on the
1435-
* non-transient relation.)
1436-
*
1437-
* Caution: the placement of this step interacts with the decision to
1438-
* handle toast rels by recursion. When we are trying to rebuild pg_class
1439-
* itself, the smgr close on pg_class must happen after all accesses in
1440-
* this function.
1441-
*/
1442-
RelationCloseSmgrByOid(r1);
1443-
RelationCloseSmgrByOid(r2);
14441425
}
14451426

14461427
/*

‎src/backend/postmaster/bgwriter.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,12 @@ BackgroundWriterMain(void)
239239
if (FirstCallSinceLastCheckpoint())
240240
{
241241
/*
242-
* After any checkpoint, close all smgr files. This is so we
243-
* won't hang onto smgr references to deleted files indefinitely.
242+
* After any checkpoint, free all smgr objects. Otherwise we
243+
* would never do so for dropped relations, as the bgwriter does
244+
* not process shared invalidation messages or call
245+
* AtEOXact_SMgr().
244246
*/
245-
smgrcloseall();
247+
smgrdestroyall();
246248
}
247249

248250
/*

‎src/backend/postmaster/checkpointer.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -462,10 +462,12 @@ CheckpointerMain(void)
462462
ckpt_performed=CreateRestartPoint(flags);
463463

464464
/*
465-
* After any checkpoint, close all smgr files. This is so we
466-
* won't hang onto smgr references to deleted files indefinitely.
465+
* After any checkpoint, free all smgr objects. Otherwise we
466+
* would never do so for dropped relations, as the checkpointer
467+
* does not process shared invalidation messages or call
468+
* AtEOXact_SMgr().
467469
*/
468-
smgrcloseall();
470+
smgrdestroyall();
469471

470472
/*
471473
* Indicate checkpoint completion to any waiting backends.
@@ -951,11 +953,8 @@ RequestCheckpoint(int flags)
951953
*/
952954
CreateCheckPoint(flags |CHECKPOINT_IMMEDIATE);
953955

954-
/*
955-
* After any checkpoint, close all smgr files. This is so we won't
956-
* hang onto smgr references to deleted files indefinitely.
957-
*/
958-
smgrcloseall();
956+
/* Free all smgr objects, as CheckpointerMain() normally would. */
957+
smgrdestroyall();
959958

960959
return;
961960
}

‎src/backend/storage/buffer/bufmgr.c

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -934,10 +934,6 @@ ExtendBufferedRelTo(BufferManagerRelation bmr,
934934
{
935935
LockRelationForExtension(bmr.rel,ExclusiveLock);
936936

937-
/* could have been closed while waiting for lock */
938-
if (bmr.rel)
939-
bmr.smgr=RelationGetSmgr(bmr.rel);
940-
941937
/* recheck, fork might have been created concurrently */
942938
if (!smgrexists(bmr.smgr,fork))
943939
smgrcreate(bmr.smgr,fork,flags&EB_PERFORMING_RECOVERY);
@@ -1897,11 +1893,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr,
18971893
* we get the lock.
18981894
*/
18991895
if (!(flags&EB_SKIP_EXTENSION_LOCK))
1900-
{
19011896
LockRelationForExtension(bmr.rel,ExclusiveLock);
1902-
if (bmr.rel)
1903-
bmr.smgr=RelationGetSmgr(bmr.rel);
1904-
}
19051897

19061898
/*
19071899
* If requested, invalidate size cache, so that smgrnblocks asks the
@@ -4155,6 +4147,7 @@ FlushRelationBuffers(Relation rel)
41554147
{
41564148
inti;
41574149
BufferDesc*bufHdr;
4150+
SMgrRelationsrel=RelationGetSmgr(rel);
41584151

41594152
if (RelationUsesLocalBuffers(rel))
41604153
{
@@ -4183,7 +4176,7 @@ FlushRelationBuffers(Relation rel)
41834176

41844177
io_start=pgstat_prepare_io_time(track_io_timing);
41854178

4186-
smgrwrite(RelationGetSmgr(rel),
4179+
smgrwrite(srel,
41874180
BufTagGetForkNum(&bufHdr->tag),
41884181
bufHdr->tag.blockNum,
41894182
localpage,
@@ -4229,7 +4222,7 @@ FlushRelationBuffers(Relation rel)
42294222
{
42304223
PinBuffer_Locked(bufHdr);
42314224
LWLockAcquire(BufferDescriptorGetContentLock(bufHdr),LW_SHARED);
4232-
FlushBuffer(bufHdr,RelationGetSmgr(rel),IOOBJECT_RELATION,IOCONTEXT_NORMAL);
4225+
FlushBuffer(bufHdr,srel,IOOBJECT_RELATION,IOCONTEXT_NORMAL);
42334226
LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
42344227
UnpinBuffer(bufHdr);
42354228
}
@@ -4442,13 +4435,17 @@ void
44424435
CreateAndCopyRelationData(RelFileLocatorsrc_rlocator,
44434436
RelFileLocatordst_rlocator,boolpermanent)
44444437
{
4445-
RelFileLocatorBackendrlocator;
44464438
charrelpersistence;
4439+
SMgrRelationsrc_rel;
4440+
SMgrRelationdst_rel;
44474441

44484442
/* Set the relpersistence. */
44494443
relpersistence=permanent ?
44504444
RELPERSISTENCE_PERMANENT :RELPERSISTENCE_UNLOGGED;
44514445

4446+
src_rel=smgropen(src_rlocator,InvalidBackendId);
4447+
dst_rel=smgropen(dst_rlocator,InvalidBackendId);
4448+
44524449
/*
44534450
* Create and copy all forks of the relation. During create database we
44544451
* have a separate cleanup mechanism which deletes complete database
@@ -4465,9 +4462,9 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator,
44654462
for (ForkNumberforkNum=MAIN_FORKNUM+1;
44664463
forkNum <=MAX_FORKNUM;forkNum++)
44674464
{
4468-
if (smgrexists(smgropen(src_rlocator,InvalidBackendId),forkNum))
4465+
if (smgrexists(src_rel,forkNum))
44694466
{
4470-
smgrcreate(smgropen(dst_rlocator,InvalidBackendId),forkNum, false);
4467+
smgrcreate(dst_rel,forkNum, false);
44714468

44724469
/*
44734470
* WAL log creation if the relation is persistent, or this is the
@@ -4481,15 +4478,6 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator,
44814478
permanent);
44824479
}
44834480
}
4484-
4485-
/* close source and destination smgr if exists. */
4486-
rlocator.backend=InvalidBackendId;
4487-
4488-
rlocator.locator=src_rlocator;
4489-
smgrcloserellocator(rlocator);
4490-
4491-
rlocator.locator=dst_rlocator;
4492-
smgrcloserellocator(rlocator);
44934481
}
44944482

44954483
/* ---------------------------------------------------------------------

‎src/backend/storage/freespace/freespace.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -532,14 +532,7 @@ fsm_readbuf(Relation rel, FSMAddress addr, bool extend)
532532
{
533533
BlockNumberblkno=fsm_logical_to_physical(addr);
534534
Bufferbuf;
535-
SMgrRelationreln;
536-
537-
/*
538-
* Caution: re-using this smgr pointer could fail if the relcache entry
539-
* gets closed. It's safe as long as we only do smgr-level operations
540-
* between here and the last use of the pointer.
541-
*/
542-
reln=RelationGetSmgr(rel);
535+
SMgrRelationreln=RelationGetSmgr(rel);
543536

544537
/*
545538
* If we haven't cached the size of the FSM yet, check it first. Also

0 commit comments

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