From d6c758c279b0116fee3532e237cf9fee65ab968b Mon Sep 17 00:00:00 2001 From: David Geier Date: Thu, 3 Sep 2026 10:38:03 +0200 Subject: [PATCH v1 4/8] Remove rd_lockinfo --- src/backend/access/common/relation.c | 2 +- src/backend/access/heap/heapam.c | 4 +- src/backend/access/index/indexam.c | 2 +- src/backend/access/nbtree/nbtutils.c | 15 ++--- src/backend/access/transam/xlogutils.c | 22 ++++--- src/backend/catalog/index.c | 4 +- src/backend/commands/indexcmds.c | 8 +-- src/backend/commands/vacuum.c | 2 +- src/backend/storage/lmgr/lmgr.c | 80 ++++++++++---------------- src/backend/utils/cache/relcache.c | 19 +----- src/include/storage/lmgr.h | 2 - src/include/utils/rel.h | 23 ++++++++ src/include/utils/rel_internal.h | 10 +--- 13 files changed, 92 insertions(+), 101 deletions(-) diff --git a/src/backend/access/common/relation.c b/src/backend/access/common/relation.c index 38b356b8239..2d0fed50a9e 100644 --- a/src/backend/access/common/relation.c +++ b/src/backend/access/common/relation.c @@ -205,7 +205,7 @@ relation_openrv_extended(const RangeVar *relation, LOCKMODE lockmode, void relation_close(Relation relation, LOCKMODE lockmode) { - LockRelId relid = relation->rd_lockInfo.lockRelId; + LockRelId relid = RelationGetLockRelId(relation); Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES); diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 72d6541734c..b507b3e996b 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -4387,8 +4387,8 @@ check_lock_if_inplace_updateable_rel(Relation relation, LOCKTAG tuptag; SET_LOCKTAG_TUPLE(tuptag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, ItemPointerGetBlockNumber(otid), ItemPointerGetOffsetNumber(otid)); if (LockHeldByMe(&tuptag, InplaceUpdateTupleLock, false)) diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index 7967e939847..ac5ef368fb2 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -177,7 +177,7 @@ try_index_open(Oid relationId, LOCKMODE lockmode) void index_close(Relation relation, LOCKMODE lockmode) { - LockRelId relid = relation->rd_lockInfo.lockRelId; + LockRelId relid = RelationGetLockRelId(relation); Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES); diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index 014faa1622f..b3032b1f1de 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -24,6 +24,7 @@ #include "common/int.h" #include "lib/qunique.h" #include "miscadmin.h" +#include "storage/lmgr.h" #include "storage/lwlock.h" #include "storage/subsystems.h" #include "utils/datum.h" @@ -448,8 +449,8 @@ _bt_vacuum_cycleid(Relation rel) { BTOneVacInfo *vac = &btvacinfo->vacuums[i]; - if (vac->relid.relId == rel->rd_lockInfo.lockRelId.relId && - vac->relid.dbId == rel->rd_lockInfo.lockRelId.dbId) + if (vac->relid.relId == RelationGetLockRelId(rel).relId && + vac->relid.dbId == RelationGetLockRelId(rel).dbId) { result = vac->cycleid; break; @@ -490,8 +491,8 @@ _bt_start_vacuum(Relation rel) for (i = 0; i < btvacinfo->num_vacuums; i++) { vac = &btvacinfo->vacuums[i]; - if (vac->relid.relId == rel->rd_lockInfo.lockRelId.relId && - vac->relid.dbId == rel->rd_lockInfo.lockRelId.dbId) + if (vac->relid.relId == RelationGetLockRelId(rel).relId && + vac->relid.dbId == RelationGetLockRelId(rel).dbId) { /* * Unlike most places in the backend, we have to explicitly @@ -512,7 +513,7 @@ _bt_start_vacuum(Relation rel) elog(ERROR, "out of btvacinfo slots"); } vac = &btvacinfo->vacuums[btvacinfo->num_vacuums]; - vac->relid = rel->rd_lockInfo.lockRelId; + vac->relid = RelationGetLockRelId(rel); vac->cycleid = result; btvacinfo->num_vacuums++; @@ -538,8 +539,8 @@ _bt_end_vacuum(Relation rel) { BTOneVacInfo *vac = &btvacinfo->vacuums[i]; - if (vac->relid.relId == rel->rd_lockInfo.lockRelId.relId && - vac->relid.dbId == rel->rd_lockInfo.lockRelId.dbId) + if (vac->relid.relId == RelationGetLockRelId(rel).relId && + vac->relid.dbId == RelationGetLockRelId(rel).dbId) { /* Remove it by shifting down the last entry */ *vac = btvacinfo->vacuums[btvacinfo->num_vacuums - 1]; diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index 58b9dab6a90..71d5b0ab2af 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -23,6 +23,7 @@ #include "access/xlogrecovery.h" #include "access/xlog_internal.h" #include "access/xlogutils.h" +#include "catalog/pg_tablespace_d.h" #include "miscadmin.h" #include "storage/fd.h" #include "storage/smgr.h" @@ -617,14 +618,21 @@ CreateFakeRelcacheEntry(RelFileLocator rlocator) sprintf(RelationGetRelationName(rel), "%u", rlocator.relNumber); /* - * We set up the lockRelId in case anything tries to lock the dummy - * relation. Note that this is fairly bogus since relNumber may be - * different from the relation's OID. It shouldn't really matter though. - * In recovery, we are running by ourselves and can't have any lock - * conflicts. While syncing, we already hold AccessExclusiveLock. + * RelationGetLockRelId() derives the LockRelId to use for locking the + * relation from rd_id and rd_rel->relisshared, in case anything tries + * to lock the dummy relation. We don't have a real OID for this fake + * entry, so use relNumber instead. Note that this is fairly bogus + * since relNumber may be different from the relation's real OID. It + * shouldn't really matter though. In recovery, we are running by + * ourselves and can't have any lock conflicts. While syncing, we + * already hold AccessExclusiveLock. Set relisshared to match rlocator + * so that RelationGetLockRelId() derives the correct dbId for shared + * relations (InvalidOid); for non-shared relations it'll derive + * MyDatabaseId, which is correct whenever this is called from a real + * backend (e.g. while syncing) and simply irrelevant during recovery. */ - rel->rd_lockInfo.lockRelId.dbId = rlocator.dbOid; - rel->rd_lockInfo.lockRelId.relId = rlocator.relNumber; + rel->rd_id = rlocator.relNumber; + rel->rd_rel->relisshared = (rlocator.spcOid == GLOBALTABLESPACE_OID); /* * Set up a non-pinned SMgrRelation reference, so that we don't need to diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index ec21b83b6b8..ce84c124cc1 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2267,9 +2267,9 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode) CacheInvalidateRelcache(userHeapRelation); /* save lockrelid and locktag for below, then close but keep locks */ - heaprelid = userHeapRelation->rd_lockInfo.lockRelId; + heaprelid = RelationGetLockRelId(userHeapRelation); SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId); - indexrelid = userIndexRelation->rd_lockInfo.lockRelId; + indexrelid = RelationGetLockRelId(userIndexRelation); table_close(userHeapRelation, NoLock); index_close(userIndexRelation, NoLock); diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5a0312fe772..b8a0c908637 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1650,7 +1650,7 @@ DefineIndex(ParseState *pstate, } /* save lockrelid and locktag for below, then close rel */ - heaprelid = rel->rd_lockInfo.lockRelId; + heaprelid = RelationGetLockRelId(rel); SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId); table_close(rel, NoLock); @@ -4177,10 +4177,10 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein * parentRelationIds built earlier. */ lockrelid = palloc_object(LockRelId); - *lockrelid = indexRel->rd_lockInfo.lockRelId; + *lockrelid = RelationGetLockRelId(indexRel); relationLocks = lappend(relationLocks, lockrelid); lockrelid = palloc_object(LockRelId); - *lockrelid = newIndexRel->rd_lockInfo.lockRelId; + *lockrelid = RelationGetLockRelId(newIndexRel); relationLocks = lappend(relationLocks, lockrelid); MemoryContextSwitchTo(oldcontext); @@ -4226,7 +4226,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein /* Add lockrelid of heap relation to the list of locked relations */ lockrelid = palloc_object(LockRelId); - *lockrelid = heapRelation->rd_lockInfo.lockRelId; + *lockrelid = RelationGetLockRelId(heapRelation); relationLocks = lappend(relationLocks, lockrelid); heaplocktag = palloc_object(LOCKTAG); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index d8c2f33c615..161f50ea560 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2202,7 +2202,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, * because the lock manager knows that both lock requests are from the * same process. */ - lockrelid = rel->rd_lockInfo.lockRelId; + lockrelid = RelationGetLockRelId(rel); LockRelationIdForSession(&lockrelid, lmode); /* diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index 2ccf7237fee..c9b6e9f04e1 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -60,26 +60,6 @@ typedef struct XactLockTableWaitInfo static void XactLockTableWaitErrorCb(void *arg); -/* - * RelationInitLockInfo - * Initializes the lock information in a relation descriptor. - * - * relcache.c must call this during creation of any reldesc. - */ -void -RelationInitLockInfo(Relation relation) -{ - Assert(RelationIsValid(relation)); - Assert(OidIsValid(RelationGetRelid(relation))); - - relation->rd_lockInfo.lockRelId.relId = RelationGetRelid(relation); - - if (relation->rd_rel->relisshared) - relation->rd_lockInfo.lockRelId.dbId = InvalidOid; - else - relation->rd_lockInfo.lockRelId.dbId = MyDatabaseId; -} - /* * SetLocktagRelationOid * Set up a locktag for a relation, given only relation OID @@ -250,8 +230,8 @@ LockRelation(Relation relation, LOCKMODE lockmode) LockAcquireResult res; SET_LOCKTAG_RELATION(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); res = LockAcquireExtended(&tag, lockmode, false, false, true, &locallock, false); @@ -282,8 +262,8 @@ ConditionalLockRelation(Relation relation, LOCKMODE lockmode) LockAcquireResult res; SET_LOCKTAG_RELATION(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); res = LockAcquireExtended(&tag, lockmode, false, true, true, &locallock, false); @@ -316,8 +296,8 @@ UnlockRelation(Relation relation, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_RELATION(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); LockRelease(&tag, lockmode, false); } @@ -336,8 +316,8 @@ CheckRelationLockedByMe(Relation relation, LOCKMODE lockmode, bool orstronger) LOCKTAG tag; SET_LOCKTAG_RELATION(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); return LockHeldByMe(&tag, lockmode, orstronger); } @@ -369,8 +349,8 @@ LockHasWaitersRelation(Relation relation, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_RELATION(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); return LockHasWaiters(&tag, lockmode, false); } @@ -426,8 +406,8 @@ LockRelationForExtension(Relation relation, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_RELATION_EXTEND(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); (void) LockAcquire(&tag, lockmode, false, false); } @@ -444,8 +424,8 @@ ConditionalLockRelationForExtension(Relation relation, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_RELATION_EXTEND(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); return (LockAcquire(&tag, lockmode, false, true) != LOCKACQUIRE_NOT_AVAIL); } @@ -461,8 +441,8 @@ RelationExtensionLockWaiterCount(Relation relation) LOCKTAG tag; SET_LOCKTAG_RELATION_EXTEND(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); return LockWaiterCount(&tag); } @@ -476,8 +456,8 @@ UnlockRelationForExtension(Relation relation, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_RELATION_EXTEND(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId); + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId); LockRelease(&tag, lockmode, false); } @@ -509,8 +489,8 @@ LockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_PAGE(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, blkno); (void) LockAcquire(&tag, lockmode, false, false); @@ -528,8 +508,8 @@ ConditionalLockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_PAGE(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, blkno); return (LockAcquire(&tag, lockmode, false, true) != LOCKACQUIRE_NOT_AVAIL); @@ -544,8 +524,8 @@ UnlockPage(Relation relation, BlockNumber blkno, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_PAGE(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, blkno); LockRelease(&tag, lockmode, false); @@ -564,8 +544,8 @@ LockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_TUPLE(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, ItemPointerGetBlockNumber(tid), ItemPointerGetOffsetNumber(tid)); @@ -585,8 +565,8 @@ ConditionalLockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE loc LOCKTAG tag; SET_LOCKTAG_TUPLE(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, ItemPointerGetBlockNumber(tid), ItemPointerGetOffsetNumber(tid)); @@ -603,8 +583,8 @@ UnlockTuple(Relation relation, const ItemPointerData *tid, LOCKMODE lockmode) LOCKTAG tag; SET_LOCKTAG_TUPLE(tag, - relation->rd_lockInfo.lockRelId.dbId, - relation->rd_lockInfo.lockRelId.relId, + RelationGetLockRelId(relation).dbId, + RelationGetLockRelId(relation).relId, ItemPointerGetBlockNumber(tid), ItemPointerGetOffsetNumber(tid)); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 3552fd88667..b33c14f5dab 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1277,11 +1277,6 @@ retry: relation->rd_rsdesc = NULL; } - /* - * initialize the relation lock manager information - */ - RelationInitLockInfo(relation); /* see lmgr.c */ - /* * initialize physical addressing information for the relation */ @@ -2023,11 +2018,6 @@ formrdesc(const char *relationName, Oid relationReltype, RelationGetRelid(relation), isshared, true); - /* - * initialize the relation lock manager information - */ - RelationInitLockInfo(relation); /* see lmgr.c */ - /* * initialize physical addressing information for the relation */ @@ -3768,8 +3758,6 @@ RelationBuildLocalRelation(const char *relname, else rel->rd_rel->relfilenode = relfilenumber; - RelationInitLockInfo(rel); /* see lmgr.c */ - RelationInitPhysicalAddr(rel); rel->rd_rel->relam = accessmtd; @@ -6599,11 +6587,10 @@ load_relcache_init_file(bool shared) rel->pgstat_info = NULL; /* - * Recompute lock and physical addressing info. This is needed in - * case the pg_internal.init file was copied from some other database - * by CREATE DATABASE. + * Recompute physical addressing info. This is needed in case the + * pg_internal.init file was copied from some other database by + * CREATE DATABASE. */ - RelationInitLockInfo(rel); RelationInitPhysicalAddr(rel); } diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h index 2a985ce5e15..64cc69c410d 100644 --- a/src/include/storage/lmgr.h +++ b/src/include/storage/lmgr.h @@ -34,8 +34,6 @@ typedef enum XLTW_Oper XLTW_RecheckExclusionConstr, } XLTW_Oper; -extern void RelationInitLockInfo(Relation relation); - /* Lock a relation */ extern void LockRelationOid(Oid relid, LOCKMODE lockmode); extern void LockRelationId(LockRelId *relid, LOCKMODE lockmode); diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 5dfbbc1715b..5c39086ca0c 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -20,6 +20,7 @@ #include "catalog/pg_class.h" #include "catalog/pg_index.h" #include "catalog/pg_publication.h" +#include "miscadmin.h" #include "nodes/bitmapset.h" #include "partitioning/partdefs.h" #include "rewrite/prs2lock.h" @@ -290,6 +291,28 @@ typedef struct ViewOptions */ #define RelationGetRelid(relation) ((relation)->rd_id) +/* + * RelationGetLockRelId + * Returns the LockRelId to use for locking the relation. + * + * This used to be a field cached in the relation descriptor + * (rd_lockInfo.lockRelId), populated once by RelationInitLockInfo(). It's + * cheap enough to compute on the fly instead, which lets us avoid storing + * it. Note that we can't derive dbId from rd_locator.dbOid: relation kinds + * without storage (e.g. partitioned tables/indexes, views) never have + * rd_locator populated, so we replicate the original relisshared-based + * computation instead. + */ +static inline LockRelId +RelationGetLockRelId(Relation relation) +{ + LockRelId lockRelId; + + lockRelId.relId = RelationGetRelid(relation); + lockRelId.dbId = relation->rd_rel->relisshared ? InvalidOid : MyDatabaseId; + return lockRelId; +} + /* * RelationGetNumberOfAttributes * Returns the total number of attributes in a relation. diff --git a/src/include/utils/rel_internal.h b/src/include/utils/rel_internal.h index 1a73fcbce42..c96b3df1f85 100644 --- a/src/include/utils/rel_internal.h +++ b/src/include/utils/rel_internal.h @@ -19,6 +19,8 @@ /* * LockRelId and LockInfo really belong to lmgr.h, but it's more convenient * to declare them here so we can have a LockInfoData field in a Relation. + * Moving them to lmgr.h would require including rel.h in lmgr.h, which creates + * a circular dependency. */ typedef struct LockRelId @@ -27,13 +29,6 @@ typedef struct LockRelId Oid dbId; /* a database identifier */ } LockRelId; -typedef struct LockInfoData -{ - LockRelId lockRelId; -} LockInfoData; - -typedef LockInfoData *LockInfo; - /* * Here are the contents of a relation cache entry. */ @@ -99,7 +94,6 @@ typedef struct RelationData SubTransactionId rd_droppedSubid; /* dropped with another Subid set */ Oid rd_id; /* relation's object id */ - LockInfoData rd_lockInfo; /* lock mgr's info for locking relation */ bool rd_islocaltemp; /* rel is a temp rel of this session */ bool rd_isnailed; /* rel is nailed in cache */ -- 2.51.0