From 94a6f126089e918bc3d08f58d64d3474c10468b3 Mon Sep 17 00:00:00 2001 From: openGaussDev Date: Wed, 9 Mar 2022 15:38:42 +0800 Subject: [PATCH] fix Offering: openGaussDev More detail: fixed the instantaneous inconsistency of data between index scan and table scan in interval partition table Signed-off-by: Match-id-a83cf32c48755b34a0443892b4875cf99e80e00c --- src/common/backend/catalog/heap.cpp | 11 ++++-- src/common/backend/catalog/pg_partition.cpp | 38 ++++++++++++++++++++ src/common/backend/parser/parse_relation.cpp | 5 +++ src/gausskernel/storage/lmgr/lmgr.cpp | 9 +++++ src/gausskernel/storage/lmgr/lock.cpp | 35 ++++++++++++++++++ src/include/catalog/pg_partition_fn.h | 15 ++++---- src/include/storage/lmgr.h | 1 + src/include/storage/lock/lock.h | 1 + 8 files changed, 104 insertions(+), 11 deletions(-) diff --git a/src/common/backend/catalog/heap.cpp b/src/common/backend/catalog/heap.cpp index 2afa519e6..2fcdf6ba7 100644 --- a/src/common/backend/catalog/heap.cpp +++ b/src/common/backend/catalog/heap.cpp @@ -6245,14 +6245,21 @@ Oid AddNewIntervalPartition(Relation rel, void* insertTuple) CacheInvalidateRelcache(rel); } + /* + * to avoid dead lock, we should release AccessShareLock on ADD_PARTITION_ACTION + * locked by the transaction before aquire AccessExclusiveLock. + */ + UnlockRelationForAccessIntervalPartTabIfHeld(rel); /* it will accept invalidation messages generated by other sessions in lockRelationForAddIntervalPartition. */ - lockRelationForAddIntervalPartition(rel); + LockRelationForAddIntervalPartition(rel); partitionRoutingForTuple(rel, insertTuple, u_sess->catalog_cxt.route); /* if the partition exists, return partition's oid */ if (u_sess->catalog_cxt.route->fileExist) { Assert(OidIsValid(u_sess->catalog_cxt.route->partitionId)); - unLockRelationForAddIntervalPartition(rel); + /* we should take AccessShareLock again before release AccessExclusiveLock for consistency. */ + LockRelationForAccessIntervalPartitionTab(rel); + UnlockRelationForAddIntervalPartition(rel); return u_sess->catalog_cxt.route->partitionId; } diff --git a/src/common/backend/catalog/pg_partition.cpp b/src/common/backend/catalog/pg_partition.cpp index dc9747fa2..7298850a4 100644 --- a/src/common/backend/catalog/pg_partition.cpp +++ b/src/common/backend/catalog/pg_partition.cpp @@ -1425,3 +1425,41 @@ Oid GetBaseRelOidOfParition(Relation relation) return relation->parentId; } +/* NB: all operations on ADD_PARTITION_ACTION sequence lock must use TopTransactionResourceOwner. */ +void LockRelationForAddIntervalPartition(Relation rel) +{ + ResourceOwner currentOwner = t_thrd.utils_cxt.CurrentResourceOwner; + t_thrd.utils_cxt.CurrentResourceOwner = t_thrd.utils_cxt.TopTransactionResourceOwner; + LockPartition(RelationGetRelid(rel), ADD_PARTITION_ACTION, + AccessExclusiveLock, PARTITION_SEQUENCE_LOCK); + t_thrd.utils_cxt.CurrentResourceOwner = currentOwner; +} + +void LockRelationForAccessIntervalPartitionTab(Relation rel) +{ + ResourceOwner currentOwner = t_thrd.utils_cxt.CurrentResourceOwner; + t_thrd.utils_cxt.CurrentResourceOwner = t_thrd.utils_cxt.TopTransactionResourceOwner; + LockPartition(RelationGetRelid(rel), ADD_PARTITION_ACTION, + AccessShareLock, PARTITION_SEQUENCE_LOCK); + t_thrd.utils_cxt.CurrentResourceOwner = currentOwner; +} + +void UnlockRelationForAccessIntervalPartTabIfHeld(Relation rel) +{ + ResourceOwner currentOwner = t_thrd.utils_cxt.CurrentResourceOwner; + t_thrd.utils_cxt.CurrentResourceOwner = t_thrd.utils_cxt.TopTransactionResourceOwner; + + UnlockPartitionSeqIfHeld(RelationGetRelid(rel), ADD_PARTITION_ACTION, AccessShareLock); + t_thrd.utils_cxt.CurrentResourceOwner = currentOwner; +} + +void UnlockRelationForAddIntervalPartition(Relation rel) +{ + ResourceOwner currentOwner = t_thrd.utils_cxt.CurrentResourceOwner; + t_thrd.utils_cxt.CurrentResourceOwner = t_thrd.utils_cxt.TopTransactionResourceOwner; + + UnlockPartition(RelationGetRelid(rel), ADD_PARTITION_ACTION, + AccessExclusiveLock, PARTITION_SEQUENCE_LOCK); + t_thrd.utils_cxt.CurrentResourceOwner = currentOwner; +} + diff --git a/src/common/backend/parser/parse_relation.cpp b/src/common/backend/parser/parse_relation.cpp index 9acfa8817..6d6c73582 100755 --- a/src/common/backend/parser/parse_relation.cpp +++ b/src/common/backend/parser/parse_relation.cpp @@ -1134,6 +1134,11 @@ Relation parserOpenTable(ParseState *pstate, const RangeVar *relation, int lockm TryUnlockAllAccounts(); } + if (rel->partMap && rel->partMap->type == PART_TYPE_INTERVAL) { + /* take AccessShareLock on ADD_PARTITION_ACTION to avoid concurrency with new partition operations. */ + LockRelationForAccessIntervalPartitionTab(rel); + } + if (IS_PGXC_COORDINATOR && !IsConnFromCoord()) { if (u_sess->attr.attr_sql.enable_parallel_ddl && !isFirstNode && isCreateView) { UnlockRelation(rel, lockmode); diff --git a/src/gausskernel/storage/lmgr/lmgr.cpp b/src/gausskernel/storage/lmgr/lmgr.cpp index 2314575b9..809db8618 100755 --- a/src/gausskernel/storage/lmgr/lmgr.cpp +++ b/src/gausskernel/storage/lmgr/lmgr.cpp @@ -1148,6 +1148,15 @@ void UnlockPartitionSeq(Oid relid, uint32 seq, LOCKMODE lockmode) (void)LockRelease(&tag, lockmode, false); } +void UnlockPartitionSeqIfHeld(Oid relid, uint32 seq, LOCKMODE lockmode) +{ + LOCKTAG tag; + + SetLocktagPartitionSeq(&tag, relid, seq); + + ReleaseLockIfHeld(&tag, lockmode, false); +} + void LockPartitionVacuum(Relation prel, Oid partId, LOCKMODE lockmode) { PartitionIdentifier* partIdentifier = NULL; diff --git a/src/gausskernel/storage/lmgr/lock.cpp b/src/gausskernel/storage/lmgr/lock.cpp index e49970366..b0dc2b148 100644 --- a/src/gausskernel/storage/lmgr/lock.cpp +++ b/src/gausskernel/storage/lmgr/lock.cpp @@ -2303,6 +2303,41 @@ void LockReleaseCurrentOwner(void) } } +/* clear all info of CurrentResourceOwner in locallock and release the lock if no other owners. */ +void ReleaseLockIfHeld(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) +{ + LOCKMETHODID lockmethodid = locktag->locktag_lockmethodid; + LockMethod lockMethodTable; + LOCALLOCKTAG localtag; + LOCALLOCK *locallock = NULL; + + CHECK_LOCKMETHODID(lockmethodid); + lockMethodTable = LockMethods[lockmethodid]; + CHECK_LOCKMODE(lockmode, lockMethodTable); + +#ifdef LOCK_DEBUG + if (LOCK_DEBUG_ENABLED(locktag)) + ereport(LOG, (errmsg("ReleaseLockIfHeld: lock [%u,%u] %s", locktag->locktag_field1, locktag->locktag_field2, + lockMethodTable->lockModeNames[lockmode]))); +#endif + + /* + * Find the LOCALLOCK entry for this lock and lockmode + */ + errno_t rc = memset_s(&localtag, sizeof(localtag), 0, sizeof(localtag)); /* must clear padding */ + securec_check(rc, "", ""); + localtag.lock = *locktag; + localtag.mode = lockmode; + + locallock = (LOCALLOCK *)hash_search(t_thrd.storage_cxt.LockMethodLocalHash, (void *)&localtag, HASH_FIND, NULL); + if ((locallock == NULL) || locallock->nLocks <= 0) { + ereport(LOG, (errmsg("you don't own a lock of type %s", lockMethodTable->lockModeNames[lockmode]))); + return; + } + + ReleaseLockIfHeld(locallock, sessionLock); +} + /* * ReleaseLockIfHeld * Release any session-level locks on this lockable object if sessionLock diff --git a/src/include/catalog/pg_partition_fn.h b/src/include/catalog/pg_partition_fn.h index 92562ebc6..0787a7b73 100644 --- a/src/include/catalog/pg_partition_fn.h +++ b/src/include/catalog/pg_partition_fn.h @@ -40,6 +40,7 @@ #include "access/htup.h" #include "storage/lock/lock.h" #include "access/heapam.h" +#include "storage/lmgr.h" #define MAX_PARTITIONKEY_NUM 4 #define MAX_PARTITION_NUM 1048575 /* update LEN_PARTITION_PREFIX as well ! */ @@ -62,16 +63,12 @@ #define ADD_PARTITION_ACTION (MAX_PARTITION_NUM + 1) /* - * We acquire a AccessExclusiveLock on ADD_PARTITION_ACTION before we decide - * to add an interval partition to prevent parallel complaints. + * We add ADD_PARTITION_ACTION sequence lock to prevent parallel complaints. */ -#define lockRelationForAddIntervalPartition(relation) \ - LockPartition(RelationGetRelid(relation), ADD_PARTITION_ACTION, \ - AccessExclusiveLock, PARTITION_SEQUENCE_LOCK); - -#define unLockRelationForAddIntervalPartition(relation) \ - UnlockPartition(RelationGetRelid(relation), ADD_PARTITION_ACTION, \ - AccessExclusiveLock, PARTITION_SEQUENCE_LOCK); +extern void LockRelationForAddIntervalPartition(Relation rel); +extern void LockRelationForAccessIntervalPartitionTab(Relation rel); +extern void UnlockRelationForAccessIntervalPartTabIfHeld(Relation rel); +extern void UnlockRelationForAddIntervalPartition(Relation rel); typedef void (*PartitionNameGetPartidCallback) (Oid partitioned_relation, const char *partition_name, Oid partId, Oid oldPartId, char partition_type, void *callback_arg, LOCKMODE callbackobj_lockMode); diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h index 3dd4b8919..288e3a6d7 100644 --- a/src/include/storage/lmgr.h +++ b/src/include/storage/lmgr.h @@ -98,6 +98,7 @@ extern void UnlockPartitionOid(Oid relid, uint32 seq, LOCKMODE lockmode); extern void LockPartitionSeq(Oid relid, uint32 seq, LOCKMODE lockmode); extern bool ConditionalLockPartitionSeq(Oid relid, uint32 seq, LOCKMODE lockmode); extern void UnlockPartitionSeq(Oid relid, uint32 seq, LOCKMODE lockmode); +extern void UnlockPartitionSeqIfHeld(Oid relid, uint32 seq, LOCKMODE lockmode); extern bool ConditionalLockPartitionWithRetry(Relation relation, Oid partitionId, LOCKMODE lockmode); diff --git a/src/include/storage/lock/lock.h b/src/include/storage/lock/lock.h index 3b6f3510b..eac174006 100644 --- a/src/include/storage/lock/lock.h +++ b/src/include/storage/lock/lock.h @@ -660,6 +660,7 @@ extern LockAcquireResult LockAcquireExtended(const LOCKTAG *locktag, LOCKMODE lo bool report_memory_error, bool allow_con_update = false, int waitSec = 0); extern void AbortStrongLockAcquire(void); extern bool LockRelease(const LOCKTAG* locktag, LOCKMODE lockmode, bool sessionLock); +extern void ReleaseLockIfHeld(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock); extern void LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks); extern void Check_FastpathBit();