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

Commit578db9c

Browse files
committed
Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.
Commit5b56264 added a comment thatSetRelationHasSubclass() callers must hold this lock. When commit17f206f extended use of this column topartitioned indexes, it didn't take the lock. As the latter commitmessage mentioned, we currently never reset a partitioned index torelhassubclass=f. That largely avoids harm from the lock omission. Thecause for fixing this now is to unblock introducing a rule about locksrequired to heap_update() a pg_class row. This might cause moredeadlocks. It gives minor user-visible benefits:- If an ALTER INDEX SET TABLESPACE runs concurrently with ALTER TABLE ATTACH PARTITION or CREATE PARTITION OF, one transaction blocks instead of failing with "tuple concurrently updated". (Many cases of DDL concurrency still fail that way.)- Match ALTER INDEX ATTACH PARTITION in choosing to lock the index.While not user-visible today, we'll need this if we ever make somethingset the flag to false for a partitioned index, like ANALYZE does todayfor tables. Back-patch to v12 (all supported versions), the plan forthe commit relying on the new rule. In back branches, addLockOrStrongerHeldByMe() instead of adding a LockHeldByMe() parameter.Reviewed (in an earlier version) by Robert Haas.Discussion:https://postgr.es/m/20240611024525.9f.nmisch@google.com
1 parentad46532 commit578db9c

File tree

7 files changed

+77
-30
lines changed

7 files changed

+77
-30
lines changed

‎src/backend/catalog/index.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,7 @@ index_create(Relation heapRelation,
10141014
if (OidIsValid(parentIndexRelid))
10151015
{
10161016
StoreSingleInheritance(indexRelationId,parentIndexRelid,1);
1017+
LockRelationOid(parentIndexRelid,ShareUpdateExclusiveLock);
10171018
SetRelationHasSubclass(parentIndexRelid, true);
10181019
}
10191020

‎src/backend/commands/indexcmds.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4245,7 +4245,10 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
42454245

42464246
/* set relhassubclass if an index partition has been added to the parent */
42474247
if (OidIsValid(parentOid))
4248+
{
4249+
LockRelationOid(parentOid,ShareUpdateExclusiveLock);
42484250
SetRelationHasSubclass(parentOid, true);
4251+
}
42494252

42504253
/* set relispartition correctly on the partition */
42514254
update_relispartition(partRelid,OidIsValid(parentOid));

‎src/backend/commands/tablecmds.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3196,8 +3196,15 @@ findAttrByName(const char *attributeName, List *schema)
31963196
* SetRelationHasSubclass
31973197
*Set the value of the relation's relhassubclass field in pg_class.
31983198
*
3199-
* NOTE: caller must be holding an appropriate lock on the relation.
3200-
* ShareUpdateExclusiveLock is sufficient.
3199+
* It's always safe to set this field to true, because all SQL commands are
3200+
* ready to see true and then find no children. On the other hand, commands
3201+
* generally assume zero children if this is false.
3202+
*
3203+
* Caller must hold any self-exclusive lock until end of transaction. If the
3204+
* new value is false, caller must have acquired that lock before reading the
3205+
* evidence that justified the false value. That way, it properly waits if
3206+
* another backend is simultaneously concluding no need to change the tuple
3207+
* (new and old values are true).
32013208
*
32023209
* NOTE: an important side-effect of this operation is that an SI invalidation
32033210
* message is sent out to all backends --- including me --- causing plans
@@ -3212,6 +3219,11 @@ SetRelationHasSubclass(Oid relationId, bool relhassubclass)
32123219
HeapTupletuple;
32133220
Form_pg_class classtuple;
32143221

3222+
Assert(CheckRelationOidLockedByMe(relationId,
3223+
ShareUpdateExclusiveLock, false) ||
3224+
CheckRelationOidLockedByMe(relationId,
3225+
ShareRowExclusiveLock, true));
3226+
32153227
/*
32163228
* Fetch a modifiable copy of the tuple, modify it, update pg_class.
32173229
*/

‎src/backend/storage/lmgr/lmgr.c

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -308,32 +308,26 @@ CheckRelationLockedByMe(Relation relation, LOCKMODE lockmode, bool orstronger)
308308
relation->rd_lockInfo.lockRelId.dbId,
309309
relation->rd_lockInfo.lockRelId.relId);
310310

311-
if (LockHeldByMe(&tag,lockmode))
312-
return true;
311+
return (orstronger ?
312+
LockOrStrongerHeldByMe(&tag,lockmode) :
313+
LockHeldByMe(&tag,lockmode));
314+
}
313315

314-
if (orstronger)
315-
{
316-
LOCKMODEslockmode;
316+
/*
317+
*CheckRelationOidLockedByMe
318+
*
319+
* Like the above, but takes an OID as argument.
320+
*/
321+
bool
322+
CheckRelationOidLockedByMe(Oidrelid,LOCKMODElockmode,boolorstronger)
323+
{
324+
LOCKTAGtag;
317325

318-
for (slockmode=lockmode+1;
319-
slockmode <=MaxLockMode;
320-
slockmode++)
321-
{
322-
if (LockHeldByMe(&tag,slockmode))
323-
{
324-
#ifdefNOT_USED
325-
/* Sometimes this might be useful for debugging purposes */
326-
elog(WARNING,"lock mode %s substituted for %s on relation %s",
327-
GetLockmodeName(tag.locktag_lockmethodid,slockmode),
328-
GetLockmodeName(tag.locktag_lockmethodid,lockmode),
329-
RelationGetRelationName(relation));
330-
#endif
331-
return true;
332-
}
333-
}
334-
}
326+
SetLocktagRelationOid(&tag,relid);
335327

336-
return false;
328+
return (orstronger ?
329+
LockOrStrongerHeldByMe(&tag,lockmode) :
330+
LockHeldByMe(&tag,lockmode));
337331
}
338332

339333
/*

‎src/backend/storage/lmgr/lock.c

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -578,11 +578,17 @@ DoLockModesConflict(LOCKMODE mode1, LOCKMODE mode2)
578578
}
579579

580580
/*
581-
* LockHeldByMe -- test whether lock 'locktag' is held with mode 'lockmode'
582-
*by the current transaction
581+
* LockHeldByMeExtended -- test whether lock 'locktag' is held by the current
582+
*transaction
583+
*
584+
* Returns true if current transaction holds a lock on 'tag' of mode
585+
* 'lockmode'. If 'orstronger' is true, a stronger lockmode is also OK.
586+
* ("Stronger" is defined as "numerically higher", which is a bit
587+
* semantically dubious but is OK for the purposes we use this for.)
583588
*/
584-
bool
585-
LockHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode)
589+
staticbool
590+
LockHeldByMeExtended(constLOCKTAG*locktag,
591+
LOCKMODElockmode,boolorstronger)
586592
{
587593
LOCALLOCKTAGlocaltag;
588594
LOCALLOCK*locallock;
@@ -598,7 +604,35 @@ LockHeldByMe(const LOCKTAG *locktag, LOCKMODE lockmode)
598604
(void*)&localtag,
599605
HASH_FIND,NULL);
600606

601-
return (locallock&&locallock->nLocks>0);
607+
if (locallock&&locallock->nLocks>0)
608+
return true;
609+
610+
if (orstronger)
611+
{
612+
LOCKMODEslockmode;
613+
614+
for (slockmode=lockmode+1;
615+
slockmode <=MaxLockMode;
616+
slockmode++)
617+
{
618+
if (LockHeldByMeExtended(locktag,slockmode, false))
619+
return true;
620+
}
621+
}
622+
623+
return false;
624+
}
625+
626+
bool
627+
LockHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode)
628+
{
629+
returnLockHeldByMeExtended(locktag,lockmode, false);
630+
}
631+
632+
bool
633+
LockOrStrongerHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode)
634+
{
635+
returnLockHeldByMeExtended(locktag,lockmode, true);
602636
}
603637

604638
#ifdefUSE_ASSERT_CHECKING

‎src/include/storage/lmgr.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ extern bool ConditionalLockRelation(Relation relation, LOCKMODE lockmode);
4747
externvoidUnlockRelation(Relationrelation,LOCKMODElockmode);
4848
externboolCheckRelationLockedByMe(Relationrelation,LOCKMODElockmode,
4949
boolorstronger);
50+
externboolCheckRelationOidLockedByMe(Oidrelid,LOCKMODElockmode,
51+
boolorstronger);
5052
externboolLockHasWaitersRelation(Relationrelation,LOCKMODElockmode);
5153

5254
externvoidLockRelationIdForSession(LockRelId*relid,LOCKMODElockmode);

‎src/include/storage/lock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,7 @@ extern void LockReleaseSession(LOCKMETHODID lockmethodid);
561561
externvoidLockReleaseCurrentOwner(LOCALLOCK**locallocks,intnlocks);
562562
externvoidLockReassignCurrentOwner(LOCALLOCK**locallocks,intnlocks);
563563
externboolLockHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode);
564+
externboolLockOrStrongerHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode);
564565
#ifdefUSE_ASSERT_CHECKING
565566
externHTAB*GetLockMethodLocalHash(void);
566567
#endif

0 commit comments

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