From c3dcd6b9f8ebf9aa2f4b054c07a31fa821bf66d4 Mon Sep 17 00:00:00 2001 From: Hadi Moshayedi Date: Tue, 9 Feb 2021 13:41:55 -0800 Subject: [PATCH] Columnar: don't include stripe reservation locks in lock graph. --- src/backend/columnar/cstore_metadata_tables.c | 43 ++++++++++++++++--- .../distributed/transaction/lock_graph.c | 12 +++++- src/include/distributed/resource_lock.h | 12 +++++- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/backend/columnar/cstore_metadata_tables.c b/src/backend/columnar/cstore_metadata_tables.c index ea1f69a2c..768b67fcc 100644 --- a/src/backend/columnar/cstore_metadata_tables.c +++ b/src/backend/columnar/cstore_metadata_tables.c @@ -30,6 +30,7 @@ #include "commands/sequence.h" #include "commands/trigger.h" #include "distributed/metadata_cache.h" +#include "distributed/resource_lock.h" #include "executor/executor.h" #include "executor/spi.h" #include "miscadmin.h" @@ -79,6 +80,8 @@ static void InsertStripeMetadataRow(uint64 storageId, StripeMetadata *stripe); static void GetHighestUsedAddressAndId(uint64 storageId, uint64 *highestUsedAddress, uint64 *highestUsedId); +static void LockForStripeReservation(Relation rel, LOCKMODE mode); +static void UnlockForStripeReservation(Relation rel, LOCKMODE mode); static List * ReadDataFileStripeList(uint64 storageId, Snapshot snapshot); static Oid ColumnarStorageIdSequenceRelationId(void); static Oid ColumnarStripeRelationId(void); @@ -727,6 +730,35 @@ GetHighestUsedAddressAndId(uint64 storageId, } +/* + * LockForStripeReservation acquires a lock for stripe reservation. + */ +static void +LockForStripeReservation(Relation rel, LOCKMODE mode) +{ + /* + * We use an advisory lock here so we can easily detect these kind of + * locks in IsProcessWaitingForSafeOperations() and don't include them + * in the lock graph. + */ + LOCKTAG tag; + SET_LOCKTAG_COLUMNAR_STRIPE_RESERVATION(tag, rel); + LockAcquire(&tag, mode, false, false); +} + + +/* + * UnlockForStripeReservation releases the stripe reservation lock. + */ +static void +UnlockForStripeReservation(Relation rel, LOCKMODE mode) +{ + LOCKTAG tag; + SET_LOCKTAG_COLUMNAR_STRIPE_RESERVATION(tag, rel); + LockRelease(&tag, mode, false); +} + + /* * ReserveStripe reserves and stripe of given size for the given relation, * and inserts it into columnar.stripe. It is guaranteed that concurrent @@ -742,16 +774,13 @@ ReserveStripe(Relation rel, uint64 sizeBytes, uint64 highestId = 0; /* - * We take ShareUpdateExclusiveLock here, so two space - * reservations conflict, space reservation <-> vacuum - * conflict, but space reservation doesn't conflict with - * reads & writes. + * We take ExclusiveLock here, so two space reservations conflict. */ - LockRelation(rel, ShareUpdateExclusiveLock); + LOCKMODE lockMode = ExclusiveLock; + LockForStripeReservation(rel, lockMode); RelFileNode relfilenode = rel->rd_node; - /* * If this is the first stripe for this relation, initialize the * metapage, otherwise use the previously initialized metapage. @@ -793,7 +822,7 @@ ReserveStripe(Relation rel, uint64 sizeBytes, InsertStripeMetadataRow(metapage->storageId, &stripe); - UnlockRelation(rel, ShareUpdateExclusiveLock); + UnlockForStripeReservation(rel, lockMode); return stripe; } diff --git a/src/backend/distributed/transaction/lock_graph.c b/src/backend/distributed/transaction/lock_graph.c index aed021ae0..8bd2150a4 100644 --- a/src/backend/distributed/transaction/lock_graph.c +++ b/src/backend/distributed/transaction/lock_graph.c @@ -25,6 +25,7 @@ #include "distributed/lock_graph.h" #include "distributed/metadata_cache.h" #include "distributed/remote_commands.h" +#include "distributed/resource_lock.h" #include "distributed/tuplestore.h" #include "storage/proc.h" #include "utils/builtins.h" @@ -471,9 +472,18 @@ IsProcessWaitingForSafeOperations(PGPROC *proc) PROCLOCK *waitProcLock = proc->waitProcLock; LOCK *waitLock = waitProcLock->tag.myLock; + /* + * Stripe reservation locks are temporary & don't hold until end of + * transaction, so we shouldn't include them in the lock graph. + */ + bool stripeReservationLock = + waitLock->tag.locktag_type == LOCKTAG_ADVISORY && + waitLock->tag.locktag_field4 == ADV_LOCKTAG_CLASS_COLUMNAR_STRIPE_RESERVATION; + return waitLock->tag.locktag_type == LOCKTAG_RELATION_EXTEND || waitLock->tag.locktag_type == LOCKTAG_PAGE || - waitLock->tag.locktag_type == LOCKTAG_SPECULATIVE_TOKEN; + waitLock->tag.locktag_type == LOCKTAG_SPECULATIVE_TOKEN || + stripeReservationLock; } diff --git a/src/include/distributed/resource_lock.h b/src/include/distributed/resource_lock.h index ddfd7c1fa..213854f8d 100644 --- a/src/include/distributed/resource_lock.h +++ b/src/include/distributed/resource_lock.h @@ -38,7 +38,10 @@ typedef enum AdvisoryLocktagClass ADV_LOCKTAG_CLASS_CITUS_JOB = 6, ADV_LOCKTAG_CLASS_CITUS_REBALANCE_COLOCATION = 7, ADV_LOCKTAG_CLASS_CITUS_COLOCATED_SHARDS_METADATA = 8, - ADV_LOCKTAG_CLASS_CITUS_OPERATIONS = 9 + ADV_LOCKTAG_CLASS_CITUS_OPERATIONS = 9, + + /* Columnar lock types */ + ADV_LOCKTAG_CLASS_COLUMNAR_STRIPE_RESERVATION = 10 } AdvisoryLocktagClass; /* CitusOperations has constants for citus operations */ @@ -98,6 +101,13 @@ typedef enum CitusOperations (uint32) operationId, \ ADV_LOCKTAG_CLASS_CITUS_OPERATIONS) +#define SET_LOCKTAG_COLUMNAR_STRIPE_RESERVATION(tag, relation) \ + SET_LOCKTAG_ADVISORY(tag, \ + relation->rd_lockInfo.lockRelId.dbId, \ + relation->rd_lockInfo.lockRelId.relId, \ + 0, \ + ADV_LOCKTAG_CLASS_COLUMNAR_STRIPE_RESERVATION) + /* Lock shard/relation metadata for safe modifications */ extern void LockShardDistributionMetadata(int64 shardId, LOCKMODE lockMode); extern void LockShardListMetadataOnWorkers(LOCKMODE lockmode, List *shardIntervalList);