From c35e22d75da4266ebd80f6ab79c07fe518741a79 Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Thu, 14 Jan 2021 21:08:26 +0300 Subject: [PATCH] Skip validation for foreign key creation commands For certaion purposes, we drop and recreate the foreign keys. As we acquire exclusive locks on the tables in between drop and re-create, we can safely skip validation phase of the foreign keys. The reason is purely being performance as foreign key validation could take a long value. --- .../distributed/commands/alter_table.c | 4 ++ ..._table_operation_for_connected_relations.c | 58 ++++++++++++++++++- .../commands/create_citus_local_table.c | 4 ++ .../commands/create_distributed_table.c | 10 +++- src/backend/distributed/commands/table.c | 11 ++-- .../distributed/commands/utility_hook.c | 3 +- src/include/distributed/commands.h | 5 +- 7 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/backend/distributed/commands/alter_table.c b/src/backend/distributed/commands/alter_table.c index 161a0980e..4378a8555 100644 --- a/src/backend/distributed/commands/alter_table.c +++ b/src/backend/distributed/commands/alter_table.c @@ -482,6 +482,10 @@ ConvertTable(TableConversionState *con) if (con->conversionType == UNDISTRIBUTE_TABLE && con->cascadeViaForeignKeys && (TableReferencing(con->relationId) || TableReferenced(con->relationId))) { + /* + * Acquire ExclusiveLock as UndistributeTable does in order to + * make sure that no modifications happen on the relations. + */ CascadeOperationForConnectedRelations(con->relationId, ExclusiveLock, CASCADE_FKEY_UNDISTRIBUTE_TABLE); diff --git a/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c b/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c index 97cfbb0c8..117eddfb3 100644 --- a/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c +++ b/src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c @@ -44,7 +44,8 @@ static char * GetDropFkeyCascadeCommand(Oid foreignKeyId); static void ExecuteCascadeOperationForRelationIdList(List *relationIdList, CascadeOperationType cascadeOperationType); - +static void ExecuteForeignKeyCreateCommand(const char *commandString, + bool skip_validation); /* * CascadeOperationForConnectedRelations executes citus table function specified @@ -106,7 +107,8 @@ CascadeOperationForConnectedRelations(Oid relationId, LOCKMODE lockMode, cascadeOperationType); /* now recreate foreign keys on tables */ - ExecuteAndLogDDLCommandList(fKeyCreationCommands); + bool skip_validation = true; + ExecuteForeignKeyCreateCommandList(fKeyCreationCommands, skip_validation); } @@ -426,3 +428,55 @@ ExecuteAndLogDDLCommand(const char *commandString) CitusProcessUtility(parseTree, commandString, PROCESS_UTILITY_TOPLEVEL, NULL, None_Receiver, NULL); } + + +/* + * ExecuteForeignKeyCreateCommandList takes a list of foreign key creation ddl commands + * and calls ExecuteAndLogForeignKeyCreateCommand function for each of them. + */ +void +ExecuteForeignKeyCreateCommandList(List *ddlCommandList, bool skip_validation) +{ + char *ddlCommand = NULL; + foreach_ptr(ddlCommand, ddlCommandList) + { + ExecuteForeignKeyCreateCommand(ddlCommand, skip_validation); + } +} + + +/* + * ExecuteForeignKeyCreateCommand takes a foreign key creation command + * and logs it in DEBUG4 log level. + * + * Then, parses, sets skip_validation flag to considering the input and + * executes the command via CitusProcessUtility. + */ +static void +ExecuteForeignKeyCreateCommand(const char *commandString, bool skip_validation) +{ + ereport(DEBUG4, (errmsg("executing foreign key create command \"%s\"", + commandString))); + + Node *parseTree = ParseTreeNode(commandString); + + /* + * We might have thrown an error if IsA(parseTree, AlterTableStmt), + * but that doesn't seem to provide any benefits, so assertion is + * fine for this case. + */ + Assert(IsA(parseTree, AlterTableStmt)); + + if (skip_validation && IsA(parseTree, AlterTableStmt)) + { + parseTree = + SkipForeignKeyValidationIfConstraintIsFkey((AlterTableStmt *) parseTree, + true); + + ereport(DEBUG4, (errmsg("skipping validation for foreign key create " + "command \"%s\"", commandString))); + } + + CitusProcessUtility(parseTree, commandString, PROCESS_UTILITY_TOPLEVEL, + NULL, None_Receiver, NULL); +} diff --git a/src/backend/distributed/commands/create_citus_local_table.c b/src/backend/distributed/commands/create_citus_local_table.c index 1700cec34..98a23e08e 100644 --- a/src/backend/distributed/commands/create_citus_local_table.c +++ b/src/backend/distributed/commands/create_citus_local_table.c @@ -138,6 +138,10 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys) bool tableHasExternalForeignKeys = TableHasExternalForeignKeys(relationId); if (tableHasExternalForeignKeys && cascadeViaForeignKeys) { + /* + * By acquiring AccessExclusiveLock, make sure that no modifications happen + * on the relations. + */ CascadeOperationForConnectedRelations(relationId, lockMode, CASCADE_FKEY_CREATE_CITUS_LOCAL_TABLE); diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index 7cddf1a19..df0a1523c 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -466,8 +466,14 @@ CreateDistributedTable(Oid relationId, Var *distributionColumn, char distributio } } - /* now recreate foreign keys that we dropped beforehand */ - ExecuteAndLogDDLCommandList(fKeyCreationCommandsRelationInvolved); + /* + * Now recreate foreign keys that we dropped beforehand. As modifications are not + * allowed on the relations that are involved in the foreign key relationship, + * we can skip the validation of the foreign keys. + */ + bool skip_validation = true; + ExecuteForeignKeyCreateCommandList(fKeyCreationCommandsRelationInvolved, + skip_validation); } diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index 0df140175..e39d3d073 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -236,7 +236,10 @@ PostprocessCreateTableStmtForeignKeys(CreateStmt *createStatement) List *relationFKeyCreationCommands = GetForeignConstraintCommandsInternal(relationId, nonDistTableFKeysFlag); DropRelationForeignKeys(relationId, nonDistTableFKeysFlag); - ExecuteAndLogDDLCommandList(relationFKeyCreationCommands); + + bool skip_validation = true; + ExecuteForeignKeyCreateCommandList(relationFKeyCreationCommands, + skip_validation); } } @@ -1103,7 +1106,8 @@ PreprocessAlterTableSchemaStmt(Node *node, const char *queryString, * ALTER TABLE ... ADD FOREIGN KEY command to skip the validation step. */ Node * -SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStatement) +SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStatement, + bool processLocalRelation) { /* first check whether a distributed relation is affected */ if (alterTableStatement->relation == NULL) @@ -1118,8 +1122,7 @@ SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStatement) return (Node *) alterTableStatement; } - bool isCitusRelation = IsCitusTable(leftRelationId); - if (!isCitusRelation) + if (!IsCitusTable(leftRelationId) && !processLocalRelation) { return (Node *) alterTableStatement; } diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 84143ad14..fd2527c73 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -416,7 +416,8 @@ multi_ProcessUtility(PlannedStmt *pstmt, * Note validation is done on the shard level when DDL propagation * is enabled. The following eagerly executes some tasks on workers. */ - parsetree = SkipForeignKeyValidationIfConstraintIsFkey(alterTableStmt); + parsetree = + SkipForeignKeyValidationIfConstraintIsFkey(alterTableStmt, false); } } } diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 50e7c1289..0f13a5e09 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -368,7 +368,8 @@ extern List * PreprocessAlterTableMoveAllStmt(Node *node, const char *queryStrin ProcessUtilityContext processUtilityContext); extern List * PreprocessAlterTableSchemaStmt(Node *node, const char *queryString, ProcessUtilityContext processUtilityContext); -extern Node * SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStmt); +extern Node * SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStmt, + bool processLocalRelation); extern bool IsAlterTableRenameStmt(RenameStmt *renameStmt); extern void ErrorIfAlterDropsPartitionColumn(AlterTableStmt *alterTableStatement); extern void PostprocessAlterTableStmt(AlterTableStmt *pStmt); @@ -481,6 +482,8 @@ extern void ErrorIfAnyPartitionRelationInvolvedInNonInheritedFKey(List *relation extern void DropRelationForeignKeys(Oid relationId, int flags); extern void ExecuteAndLogDDLCommandList(List *ddlCommandList); extern void ExecuteAndLogDDLCommand(const char *commandString); +extern void ExecuteForeignKeyCreateCommandList(List *ddlCommandList, + bool skip_validation); /* create_citus_local_table.c */ extern void CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys);