external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Introduce GUC citus.skip_constraint_validation (#6281) 

Introduces a new GUC named citus.skip_constraint_validation, which basically skips constraint validation when set to on.
For some several places that we hack to skip the foreign key validation phase, now we use this GUC.
pull/6307/head
Ahmet Gedemenli 2022-09-08 18:13:18 +03:00 committed by GitHub
parent 79ba490b1f
commit eadc88a800
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 127 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/backend/distributed/commands/cascade_table_operation_for_connected_relations.c
View File

@ -623,11 +623,11 @@ ExecuteForeignKeyCreateCommand(const char *commandString, bool skip_validation)
*/
Assert(IsA(parseTree, AlterTableStmt));
bool oldSkipConstraintsValidationValue = SkipConstraintValidation;
if (skip_validation && IsA(parseTree, AlterTableStmt))
{
parseTree =
SkipForeignKeyValidationIfConstraintIsFkey((AlterTableStmt *) parseTree,
true);
EnableSkippingConstraintValidation();
ereport(DEBUG4, (errmsg("skipping validation for foreign key create "
"command \"%s\"", commandString)));
@ -635,4 +635,6 @@ ExecuteForeignKeyCreateCommand(const char *commandString, bool skip_validation)
ProcessUtilityParseTree(parseTree, commandString, PROCESS_UTILITY_QUERY,
NULL, None_Receiver, NULL);
SkipConstraintValidation = oldSkipConstraintsValidationValue;
}

93
src/backend/distributed/commands/foreign_constraint.c
View File

@ -30,6 +30,7 @@
#include "distributed/namespace_utils.h"
#include "distributed/reference_table_utils.h"
#include "distributed/version_compat.h"
#include "miscadmin.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/inval.h"
@ -78,7 +79,6 @@ static List * GetForeignKeyIdsForColumn(char *columnName, Oid relationId,
int searchForeignKeyColumnFlags);
static List * GetForeignKeysWithLocalTables(Oid relationId);
static bool IsTableTypeIncluded(Oid relationId, int flags);
static void UpdateConstraintIsValid(Oid constraintId, bool isValid);
/*
@ -1197,15 +1197,12 @@ IsTableTypeIncluded(Oid relationId, int flags)
* returns the list of commands that are required to create the foreign
* constraints for that shardInterval.
*
* The function does the following hack:
* - Create the foreign constraints as INVALID on the shards
* - Manually update pg_constraint to mark the same foreign
* constraints as VALID
* The function adds a "SET LOCAL citus.skip_constraint_validation TO ON"
* command to command list, to prevent the validation for foreign keys.
*
* We implement the above hack because we aim to skip the validation phase
* of foreign keys to reference tables. The validation is pretty costly and
* given that the source placements already valid, the validation in the
* target nodes is useless.
* We skip the validation phase of foreign keys to reference tables because
* the validation is pretty costly and given that the source placements are
* already valid, the validation in the target nodes is useless.
*
* The function does not apply the same logic for the already invalid foreign
* constraints.
@ -1218,7 +1215,7 @@ GetForeignConstraintCommandsToReferenceTable(ShardInterval *shardInterval)
uint64 shardId = shardInterval->shardId;
Oid relationId = shardInterval->relationId;
List *commandList = NIL;
List *commandList = list_make1("SET LOCAL citus.skip_constraint_validation TO ON;");
/*
* Set search_path to NIL so that all objects outside of pg_catalog will be
@ -1243,8 +1240,6 @@ GetForeignConstraintCommandsToReferenceTable(ShardInterval *shardInterval)
while (HeapTupleIsValid(heapTuple))
{
Form_pg_constraint constraintForm = (Form_pg_constraint) GETSTRUCT(heapTuple);
char *constraintDefinition = NULL;
if (constraintForm->contype != CONSTRAINT_FOREIGN)
{
@ -1272,22 +1267,7 @@ GetForeignConstraintCommandsToReferenceTable(ShardInterval *shardInterval)
char *schemaName = get_namespace_name(schemaId);
char *escapedSchemaName = quote_literal_cstr(schemaName);
/*
* We're first marking the constraint's valid field as invalid
* and get the constraint definition. Later, we mark the constraint
* as valid back with directly updating to pg_constraint.
*/
if (constraintForm->convalidated == true)
{
UpdateConstraintIsValid(constraintId, false);
constraintDefinition = pg_get_constraintdef_command(constraintId);
UpdateConstraintIsValid(constraintId, true);
}
else
{
/* if the constraint is not valid, simply do nothing special */
constraintDefinition = pg_get_constraintdef_command(constraintId);
}
char *constraintDefinition = pg_get_constraintdef_command(constraintId);
StringInfo applyForeignConstraintCommand = makeStringInfo();
appendStringInfo(applyForeignConstraintCommand,
@ -1295,6 +1275,7 @@ GetForeignConstraintCommandsToReferenceTable(ShardInterval *shardInterval)
escapedSchemaName, referencedShardId,
escapedReferencedSchemaName,
quote_literal_cstr(constraintDefinition));
commandList = lappend(commandList, applyForeignConstraintCommand->data);
/* mark the constraint as valid again on the shard */
@ -1325,60 +1306,22 @@ GetForeignConstraintCommandsToReferenceTable(ShardInterval *shardInterval)
/* revert back to original search_path */
PopOverrideSearchPath();
commandList = lappend(commandList, "RESET citus.skip_constraint_validation;");
return commandList;
}
/*
* UpdateConstraintIsValid is a utility function with sets the
* pg_constraint.convalidated to the given isValid for the given
* constraintId.
*
* This function should be called with caution because if used wrong
* could lead to data inconsistencies.
* EnableSkippingConstraintValidation is simply a C interface for setting the following:
* SET LOCAL citus.skip_constraint_validation TO on;
*/
static void
UpdateConstraintIsValid(Oid constraintId, bool isValid)
void
EnableSkippingConstraintValidation()
{
ScanKeyData scankey[1];
Relation pgConstraint = table_open(ConstraintRelationId, AccessShareLock);
TupleDesc tupleDescriptor = RelationGetDescr(pgConstraint);
Datum values[Natts_pg_constraint];
bool isnull[Natts_pg_constraint];
bool replace[Natts_pg_constraint];
ScanKeyInit(&scankey[0],
Anum_pg_constraint_oid,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(constraintId));
SysScanDesc scanDescriptor = systable_beginscan(pgConstraint,
ConstraintOidIndexId,
true,
NULL,
1,
scankey);
HeapTuple heapTuple = systable_getnext(scanDescriptor);
if (!HeapTupleIsValid(heapTuple))
{
elog(ERROR, "could not find tuple for constraint %u", constraintId);
}
memset(replace, 0, sizeof(replace));
values[Anum_pg_constraint_convalidated - 1] = BoolGetDatum(isValid);
isnull[Anum_pg_constraint_convalidated - 1] = false;
replace[Anum_pg_constraint_convalidated - 1] = true;
heapTuple = heap_modify_tuple(heapTuple, tupleDescriptor, values, isnull, replace);
CatalogTupleUpdate(pgConstraint, &heapTuple->t_self, heapTuple);
CacheInvalidateHeapTuple(pgConstraint, heapTuple, NULL);
CommandCounterIncrement();
systable_endscan(scanDescriptor);
table_close(pgConstraint, NoLock);
set_config_option("citus.skip_constraint_validation", "true",
PGC_SUSET, PGC_S_SESSION,
GUC_ACTION_LOCAL, true, 0, false);
}

30
src/backend/distributed/commands/table.c
View File

@ -1854,52 +1854,44 @@ PreprocessAlterTableSchemaStmt(Node *node, const char *queryString,
* statement to be worked on the distributed table. Currently, it only processes
* ALTER TABLE ... ADD FOREIGN KEY command to skip the validation step.
*/
Node *
SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStatement,
bool processLocalRelation)
void
SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStatement)
{
/* first check whether a distributed relation is affected */
if (alterTableStatement->relation == NULL)
{
return (Node *) alterTableStatement;
return;
}
LOCKMODE lockmode = AlterTableGetLockLevel(alterTableStatement->cmds);
Oid leftRelationId = AlterTableLookupRelation(alterTableStatement, lockmode);
if (!OidIsValid(leftRelationId))
{
return (Node *) alterTableStatement;
return;
}
if (!IsCitusTable(leftRelationId) && !processLocalRelation)
if (!IsCitusTable(leftRelationId))
{
return (Node *) alterTableStatement;
return;
}
/*
* We check if there is a ADD FOREIGN CONSTRAINT command in sub commands list.
* If there is we assign referenced releation id to rightRelationId and we also
* set skip_validation to true to prevent PostgreSQL to verify validity of the
* foreign constraint in master. Validity will be checked in workers anyway.
*/
List *commandList = alterTableStatement->cmds;
AlterTableCmd *command = NULL;
foreach_ptr(command, commandList)
foreach_ptr(command, alterTableStatement->cmds)
{
AlterTableType alterTableType = command->subtype;
if (alterTableType == AT_AddConstraint)
{
/* skip only if the constraint is a foreign key */
Constraint *constraint = (Constraint *) command->def;
if (constraint->contype == CONSTR_FOREIGN)
{
/* foreign constraint validations will be done in shards. */
constraint->skip_validation = true;
/* set the GUC skip_constraint_validation to on */
EnableSkippingConstraintValidation();
return;
}
}
}
return (Node *) alterTableStatement;
}

10
src/backend/distributed/commands/utility_hook.c
View File

@ -378,6 +378,7 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
Node *parsetree = pstmt->utilityStmt;
List *ddlJobs = NIL;
DistOpsValidationState distOpsValidationState = HasNoneValidObject;
bool oldSkipConstraintsValidationValue = SkipConstraintValidation;
if (IsA(parsetree, ExplainStmt) &&
IsA(((ExplainStmt *) parsetree)->query, Query))
@ -596,9 +597,7 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
* Citus intervening. The only exception is partition column drop, in
* which case we error out. Advanced Citus users use this to implement their
* own DDL propagation. We also use it to avoid re-propagating DDL commands
* when changing MX tables on workers. Below, we also make sure that DDL
* commands don't run queries that might get intercepted by Citus and error
* out, specifically we skip validation in foreign keys.
* when changing MX tables on workers.
*/
if (IsA(parsetree, AlterTableStmt))
@ -617,8 +616,7 @@ ProcessUtilityInternal(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, false);
SkipForeignKeyValidationIfConstraintIsFkey(alterTableStmt);
}
}
}
@ -904,6 +902,8 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
*/
CitusHasBeenLoaded(); /* lgtm[cpp/return-value-ignored] */
}
SkipConstraintValidation = oldSkipConstraintsValidationValue;
}

10
src/backend/distributed/executor/multi_executor.c
View File

@ -64,6 +64,11 @@ int MultiShardConnectionType = PARALLEL_CONNECTION;
bool WritableStandbyCoordinator = false;
bool AllowModificationsFromWorkersToReplicatedTables = true;
/*
* Controlled by the GUC citus.skip_constraint_validation
*/
bool SkipConstraintValidation = false;
/*
* Setting that controls whether distributed queries should be
* allowed within a task execution.
@ -856,6 +861,11 @@ AlterTableConstraintCheck(QueryDesc *queryDesc)
return false;
}
if (SkipConstraintValidation)
{
return true;
}
/*
* While an ALTER TABLE is in progress, we might do SELECTs on some
* catalog tables too. For example, when dropping a column, citus_drop_trigger()

15
src/backend/distributed/shared_library_init.c
View File

@ -2139,6 +2139,21 @@ RegisterCitusConfigVariables(void)
GUC_NO_SHOW_ALL,
NULL, NULL, NULL);
DefineCustomBoolVariable(
"citus.skip_constraint_validation",
gettext_noop("Skip validation of constraints"),
gettext_noop("Validating constraints is a costly operation which effects Citus' "
"performance negatively. With this GUC set to true, we skip "
"validating them. Constraint validation can be redundant for some "
"cases. For instance, when moving a shard, which has already "
"validated constraints at the source; we don't need to validate "
"the constraints again at the destination."),
&SkipConstraintValidation,
false,
PGC_SUSET,
0,
NULL, NULL, NULL);
DefineCustomBoolVariable(
"citus.skip_jsonb_validation_in_copy",
gettext_noop("Skip validation of JSONB columns on the coordinator during COPY "

4
src/include/distributed/commands.h
View File

@ -285,6 +285,7 @@ extern bool TableHasExternalForeignKeys(Oid relationId);
extern List * GetForeignKeyOids(Oid relationId, int flags);
extern Oid GetReferencedTableId(Oid foreignKeyId);
extern Oid GetReferencingTableId(Oid foreignKeyId);
extern void EnableSkippingConstraintValidation(void);
extern bool RelationInvolvedInAnyNonInheritedForeignKeys(Oid relationId);
@ -529,8 +530,7 @@ 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,
bool processLocalRelation);
extern void SkipForeignKeyValidationIfConstraintIsFkey(AlterTableStmt *alterTableStmt);
extern bool IsAlterTableRenameStmt(RenameStmt *renameStmt);
extern void ErrorIfAlterDropsPartitionColumn(AlterTableStmt *alterTableStatement);
extern void PostprocessAlterTableStmt(AlterTableStmt *pStmt);

1
src/include/distributed/multi_executor.h
View File

@ -62,6 +62,7 @@ typedef struct TransactionProperties
extern bool AllowNestedDistributedExecution;
extern bool SkipConstraintValidation;
extern int MultiShardConnectionType;
extern bool WritableStandbyCoordinator;
extern bool AllowModificationsFromWorkersToReplicatedTables;

49
src/test/regress/expected/foreign_key_to_reference_shard_rebalance.out
View File

@ -171,14 +171,16 @@ ALTER TABLE referenceing_dist_table ADD CONSTRAINT c2 FOREIGN KEY (col2) REFEREN
ALTER TABLE referenceing_dist_table ADD CONSTRAINT very_very_very_very_very_very_very_very_very_very_very_very_very_long FOREIGN KEY (col3) REFERENCES reference_table_commands(id) ON UPDATE CASCADE;
NOTICE: identifier "very_very_very_very_very_very_very_very_very_very_very_very_very_long" will be truncated to "very_very_very_very_very_very_very_very_very_very_very_very_ver"
SELECT * FROM get_foreign_key_to_reference_table_commands('referenceing_dist_table'::regclass);
get_foreign_key_to_reference_table_commands
get_foreign_key_to_reference_table_commands
---------------------------------------------------------------------
SELECT worker_apply_inter_shard_ddl_command (15000018, 'fkey_to_reference_shard_rebalance', 15000017, 'fkey_to_reference_shard_rebalance', 'ALTER TABLE fkey_to_reference_shard_rebalance.referenceing_dist_table ADD CONSTRAINT c1 FOREIGN KEY (col1) REFERENCES fkey_to_reference_shard_rebalance.reference_table_commands(id) ON UPDATE CASCADE NOT VALID')
SET LOCAL citus.skip_constraint_validation TO ON;
SELECT worker_apply_inter_shard_ddl_command (15000018, 'fkey_to_reference_shard_rebalance', 15000017, 'fkey_to_reference_shard_rebalance', 'ALTER TABLE fkey_to_reference_shard_rebalance.referenceing_dist_table ADD CONSTRAINT c1 FOREIGN KEY (col1) REFERENCES fkey_to_reference_shard_rebalance.reference_table_commands(id) ON UPDATE CASCADE')
UPDATE pg_constraint SET convalidated = true WHERE conrelid = 'fkey_to_reference_shard_rebalance.referenceing_dist_table_15000018'::regclass AND conname = 'c1_15000018'
SELECT worker_apply_inter_shard_ddl_command (15000018, 'fkey_to_reference_shard_rebalance', 15000017, 'fkey_to_reference_shard_rebalance', 'ALTER TABLE fkey_to_reference_shard_rebalance.referenceing_dist_table ADD CONSTRAINT c2 FOREIGN KEY (col2) REFERENCES fkey_to_reference_shard_rebalance.reference_table_commands(id) ON UPDATE CASCADE NOT VALID')
SELECT worker_apply_inter_shard_ddl_command (15000018, 'fkey_to_reference_shard_rebalance', 15000017, 'fkey_to_reference_shard_rebalance', 'ALTER TABLE fkey_to_reference_shard_rebalance.referenceing_dist_table ADD CONSTRAINT very_very_very_very_very_very_very_very_very_very_very_very_ver FOREIGN KEY (col3) REFERENCES fkey_to_reference_shard_rebalance.reference_table_commands(id) ON UPDATE CASCADE NOT VALID')
SELECT worker_apply_inter_shard_ddl_command (15000018, 'fkey_to_reference_shard_rebalance', 15000017, 'fkey_to_reference_shard_rebalance', 'ALTER TABLE fkey_to_reference_shard_rebalance.referenceing_dist_table ADD CONSTRAINT very_very_very_very_very_very_very_very_very_very_very_very_ver FOREIGN KEY (col3) REFERENCES fkey_to_reference_shard_rebalance.reference_table_commands(id) ON UPDATE CASCADE')
UPDATE pg_constraint SET convalidated = true WHERE conrelid = 'fkey_to_reference_shard_rebalance.referenceing_dist_table_15000018'::regclass AND conname = 'very_very_very_very_very_very_very_very_very__754e8716_15000018'
(5 rows)
RESET citus.skip_constraint_validation;
(7 rows)
-- and show that rebalancer works fine
SELECT master_move_shard_placement(15000018, 'localhost', :worker_1_port, 'localhost', :worker_2_port, 'force_logical');
@ -197,13 +199,34 @@ SELECT conname, contype, convalidated FROM pg_constraint WHERE conrelid = 'fkey_
(3 rows)
\c - - - :master_port
SET search_path TO fkey_to_reference_shard_rebalance;
SET citus.shard_replication_factor to 1;
-- test moving a shard with foreign key
create table ref_table_with_fkey (id int primary key);
select create_reference_table('ref_table_with_fkey');
create_reference_table
---------------------------------------------------------------------
(1 row)
insert into ref_table_with_fkey select s from generate_series(0,9) s;
create table partitioned_tbl_with_fkey (x int, y int, t timestamptz default now()) partition by range (t);
select create_distributed_table('partitioned_tbl_with_fkey','x');
create_distributed_table
---------------------------------------------------------------------
(1 row)
create table partition_1_with_fkey partition of partitioned_tbl_with_fkey for values from ('2022-01-01') to ('2022-12-31');
create table partition_2_with_fkey partition of partitioned_tbl_with_fkey for values from ('2023-01-01') to ('2023-12-31');
insert into partitioned_tbl_with_fkey (x,y) select s,s%10 from generate_series(1,100) s;
ALTER TABLE partitioned_tbl_with_fkey ADD CONSTRAINT fkey_to_ref_tbl FOREIGN KEY (y) REFERENCES ref_table_with_fkey(id);
WITH shardid AS (SELECT shardid FROM pg_dist_shard where logicalrelid = 'partitioned_tbl_with_fkey'::regclass ORDER BY shardid LIMIT 1)
SELECT citus_move_shard_placement(shardid.shardid, 'localhost', 57637, 'localhost', 57638, shard_transfer_mode := 'force_logical') FROM shardid;
citus_move_shard_placement
---------------------------------------------------------------------
(1 row)
SET client_min_messages TO WARNING;
DROP SCHEMA fkey_to_reference_shard_rebalance CASCADE;
NOTICE: drop cascades to 8 other objects
DETAIL: drop cascades to type fkey_to_reference_shard_rebalance.foreign_details
drop cascades to view fkey_to_reference_shard_rebalance.table_fkeys_in_workers
drop cascades to table fkey_to_reference_shard_rebalance.referenced_table
drop cascades to table fkey_to_reference_shard_rebalance.referencing_table
drop cascades to table fkey_to_reference_shard_rebalance.referencing_table2
drop cascades to function fkey_to_reference_shard_rebalance.get_foreign_key_to_reference_table_commands(oid)
drop cascades to table fkey_to_reference_shard_rebalance.reference_table_commands
drop cascades to table fkey_to_reference_shard_rebalance.referenceing_dist_table

4
src/test/regress/expected/multi_foreign_key.out
View File

@ -431,10 +431,10 @@ SELECT create_distributed_table('referencing_table', 'ref_id', 'hash');
(1 row)
-- verify that we skip foreign key validation when propagation is turned off
-- verify that we skip foreign key validation when citus.skip_constraint_validation is set to ON
-- not skipping validation would result in a distributed query, which emits debug messages
BEGIN;
SET LOCAL citus.enable_ddl_propagation TO off;
SET LOCAL citus.skip_constraint_validation TO on;
SET LOCAL client_min_messages TO DEBUG1;
ALTER TABLE referencing_table ADD CONSTRAINT test_constraint FOREIGN KEY (ref_id) REFERENCES referenced_table (id);
ABORT;

20
src/test/regress/sql/foreign_key_to_reference_shard_rebalance.sql
View File

@ -79,4 +79,24 @@ SELECT conname, contype, convalidated FROM pg_constraint WHERE conrelid = 'fkey_
\c - - - :master_port
SET search_path TO fkey_to_reference_shard_rebalance;
SET citus.shard_replication_factor to 1;
-- test moving a shard with foreign key
create table ref_table_with_fkey (id int primary key);
select create_reference_table('ref_table_with_fkey');
insert into ref_table_with_fkey select s from generate_series(0,9) s;
create table partitioned_tbl_with_fkey (x int, y int, t timestamptz default now()) partition by range (t);
select create_distributed_table('partitioned_tbl_with_fkey','x');
create table partition_1_with_fkey partition of partitioned_tbl_with_fkey for values from ('2022-01-01') to ('2022-12-31');
create table partition_2_with_fkey partition of partitioned_tbl_with_fkey for values from ('2023-01-01') to ('2023-12-31');
insert into partitioned_tbl_with_fkey (x,y) select s,s%10 from generate_series(1,100) s;
ALTER TABLE partitioned_tbl_with_fkey ADD CONSTRAINT fkey_to_ref_tbl FOREIGN KEY (y) REFERENCES ref_table_with_fkey(id);
WITH shardid AS (SELECT shardid FROM pg_dist_shard where logicalrelid = 'partitioned_tbl_with_fkey'::regclass ORDER BY shardid LIMIT 1)
SELECT citus_move_shard_placement(shardid.shardid, 'localhost', 57637, 'localhost', 57638, shard_transfer_mode := 'force_logical') FROM shardid;
SET client_min_messages TO WARNING;
DROP SCHEMA fkey_to_reference_shard_rebalance CASCADE;

4
src/test/regress/sql/multi_foreign_key.sql
View File

@ -246,10 +246,10 @@ SELECT create_distributed_table('referenced_table', 'id', 'hash');
CREATE TABLE referencing_table(id int, ref_id int);
SELECT create_distributed_table('referencing_table', 'ref_id', 'hash');
-- verify that we skip foreign key validation when propagation is turned off
-- verify that we skip foreign key validation when citus.skip_constraint_validation is set to ON
-- not skipping validation would result in a distributed query, which emits debug messages
BEGIN;
SET LOCAL citus.enable_ddl_propagation TO off;
SET LOCAL citus.skip_constraint_validation TO on;
SET LOCAL client_min_messages TO DEBUG1;
ALTER TABLE referencing_table ADD CONSTRAINT test_constraint FOREIGN KEY (ref_id) REFERENCES referenced_table (id);
ABORT;