diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index 55b5ae30b..0c2172255 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -382,6 +382,15 @@ CreateDistributedTable(Oid relationId, Var *distributionColumn, char distributio CreateTableMetadataOnWorkers(relationId); } + /* + * We've a custom way of foreign key graph invalidation, + * see InvalidateForeignKeyGraph(). + */ + if (TableReferenced(relationId) || TableReferencing(relationId)) + { + InvalidateForeignKeyGraph(); + } + /* if this table is partitioned table, distribute its partitions too */ if (PartitionedTable(relationId)) { @@ -761,13 +770,8 @@ EnsureRelationCanBeDistributed(Oid relationId, Var *distributionColumn, ErrorIfUnsupportedConstraint(relation, distributionMethod, distributionColumn, colocationId); - if (TableReferenced(relationId) || TableReferencing(relationId)) - { - InvalidateForeignKeyGraph(); - } ErrorIfUnsupportedPolicy(relation); - relation_close(relation, NoLock); } diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 848524f93..04842ee43 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -178,7 +178,7 @@ static void InvalidateForeignKeyGraphForDDL(void); * We need to run some of the commands sequentially if there is a foreign constraint * from/to reference table. */ -static bool ShouldExecuteAlterTableSequentially(Oid relationId, AlterTableCmd *command); +static bool SetupExecutionModeForAlterTable(Oid relationId, AlterTableCmd *command); /* @@ -1450,8 +1450,8 @@ PlanAlterTableStmt(AlterTableStmt *alterTableStatement, const char *alterTableCo rightRelationId = RangeVarGetRelid(partitionCommand->name, NoLock, false); } #endif - executeSequentially |= ShouldExecuteAlterTableSequentially(leftRelationId, - command); + executeSequentially |= SetupExecutionModeForAlterTable(leftRelationId, + command); } ddlJob = palloc0(sizeof(DDLJob)); @@ -4002,17 +4002,30 @@ ProcessDropTableStmt(DropStmt *dropTableStatement) /* - * ShouldExecuteAlterTableSequentially checks if the given ALTER TABLE - * statements should be executed sequentially when there is a foreign - * constraint from a distributed table to a reference table. - * In case of a column related ALTER TABLE operation, we check explicitly - * if there is a foreign constraint on this column from/to a reference table. - * Additionally, if the command is run inside a transaction block, we call - * SetLocalMultiShardModifyModeToSequential so that the further commands - * in the same transaction uses the same connections and does not error out. + * SetupExecutionModeForAlterTable is the function that is responsible + * for two things for practial purpose for not doing the same checks + * twice: + * (a) For any command, decide and return whether we should + * run the command in sequntial mode + * (b) For commands in a transaction block, set the transaction local + * multi-shard modify mode to sequential when necessary + * + * The commands that operate on the same reference table shard in parallel + * is in the interest of (a), where the return value indicates the executor + * to run the command sequentially to prevent self-deadlocks. + * + * The commands that both operate on the same reference table shard in parallel + * and cascades to run any parallel operation is in the interest of (b). By + * setting the multi-shard mode, we ensure that the cascading parallel commands + * are executed sequentially to prevent self-deadlocks. + * + * One final note on the function is that if the function decides to execute + * the command in sequential mode, and a parallel command has already been + * executed in the same transaction, the function errors out. See the comment + * in the function for the rationale. */ static bool -ShouldExecuteAlterTableSequentially(Oid relationId, AlterTableCmd *command) +SetupExecutionModeForAlterTable(Oid relationId, AlterTableCmd *command) { bool executeSequentially = false; AlterTableType alterTableType = command->subtype; diff --git a/src/backend/distributed/master/master_repair_shards.c b/src/backend/distributed/master/master_repair_shards.c index 4d56350d8..40ecce5a4 100644 --- a/src/backend/distributed/master/master_repair_shards.c +++ b/src/backend/distributed/master/master_repair_shards.c @@ -419,10 +419,7 @@ CopyShardForeignConstraintCommandListGrouped(ShardInterval *shardInterval, if (PartitionMethod(referencedRelationId) == DISTRIBUTE_BY_NONE) { - List *shardList = LoadShardList(referencedRelationId); - uint64 *shardIdPointer = (uint64 *) linitial(shardList); - - referencedShardId = (*shardIdPointer); + referencedShardId = GetFirstShardId(referencedRelationId); } else { @@ -453,6 +450,21 @@ CopyShardForeignConstraintCommandListGrouped(ShardInterval *shardInterval, } +/* + * GetFirstShardId is a helper function which returns the first + * shardId of the given distributed relation. The function doesn't + * sort the shardIds, so it is mostly useful for reference tables. + */ +uint64 +GetFirstShardId(Oid relationId) +{ + List *shardList = LoadShardList(relationId); + uint64 *shardIdPointer = (uint64 *) linitial(shardList); + + return (*shardIdPointer); +} + + /* * ConstuctQualifiedShardName creates the fully qualified name string of the * given shard in ._ format. diff --git a/src/backend/distributed/master/master_stage_protocol.c b/src/backend/distributed/master/master_stage_protocol.c index 6129931cb..9a21ff820 100644 --- a/src/backend/distributed/master/master_stage_protocol.c +++ b/src/backend/distributed/master/master_stage_protocol.c @@ -673,10 +673,7 @@ WorkerCreateShard(Oid relationId, int shardIndex, uint64 shardId, List *ddlComma } else if (PartitionMethod(referencedRelationId) == DISTRIBUTE_BY_NONE) { - List *shardList = LoadShardList(referencedRelationId); - uint64 *shardIdPointer = (uint64 *) linitial(shardList); - - referencedShardId = (*shardIdPointer); + referencedShardId = GetFirstShardId(referencedRelationId); } else { diff --git a/src/backend/distributed/transaction/relation_access_tracking.c b/src/backend/distributed/transaction/relation_access_tracking.c index 25dcab3c0..2f41eaa57 100644 --- a/src/backend/distributed/transaction/relation_access_tracking.c +++ b/src/backend/distributed/transaction/relation_access_tracking.c @@ -33,6 +33,8 @@ bool EnforceForeignKeyRestrictions = true; #define PARALLEL_MODE_FLAG_OFFSET 3 + +/* simply set parallel bits as defined below for select, dml and ddl */ #define PARALLEL_ACCESS_MASK (int) (0 | \ (1 << (PLACEMENT_ACCESS_SELECT + \ PARALLEL_MODE_FLAG_OFFSET)) | \ diff --git a/src/include/distributed/master_metadata_utility.h b/src/include/distributed/master_metadata_utility.h index 6133b9a5f..5f63392b4 100644 --- a/src/include/distributed/master_metadata_utility.h +++ b/src/include/distributed/master_metadata_utility.h @@ -159,6 +159,7 @@ extern void EnsureSuperUser(void); extern void EnsureReplicationSettings(Oid relationId, char replicationModel); extern bool RegularTable(Oid relationId); extern char * ConstructQualifiedShardName(ShardInterval *shardInterval); +extern uint64 GetFirstShardId(Oid relationId); extern Datum StringToDatum(char *inputString, Oid dataType); extern char * DatumToString(Datum datum, Oid dataType); diff --git a/src/test/regress/expected/foreign_key_restriction_enforcement.out b/src/test/regress/expected/foreign_key_restriction_enforcement.out index bcc7df753..18fc03d58 100644 --- a/src/test/regress/expected/foreign_key_restriction_enforcement.out +++ b/src/test/regress/expected/foreign_key_restriction_enforcement.out @@ -1001,12 +1001,12 @@ DEBUG: schema "test_fkey_to_ref_in_tx" already exists, skipping DETAIL: NOTICE from localhost:57638 DEBUG: schema "test_fkey_to_ref_in_tx" already exists, skipping DETAIL: NOTICE from localhost:57637 -DEBUG: switching to sequential query execution mode -DETAIL: Reference relation "test_table_1" is modified, which might lead to data inconsistencies or distributed deadlocks via parallel accesses to hash distributed relations due to foreign keys. Any parallel modification to those hash distributed relations in the same transaction can only be executed in sequential query execution mode DEBUG: schema "test_fkey_to_ref_in_tx" already exists, skipping DETAIL: NOTICE from localhost:57637 DEBUG: schema "test_fkey_to_ref_in_tx" already exists, skipping DETAIL: NOTICE from localhost:57638 +DEBUG: switching to sequential query execution mode +DETAIL: Reference relation "test_table_1" is modified, which might lead to data inconsistencies or distributed deadlocks via parallel accesses to hash distributed relations due to foreign keys. Any parallel modification to those hash distributed relations in the same transaction can only be executed in sequential query execution mode NOTICE: Copying data from local table... DEBUG: Copied 101 rows create_reference_table