From e00d1546f374e93f0a5e059565a8fb125a1ef136 Mon Sep 17 00:00:00 2001 From: Hadi Moshayedi Date: Fri, 1 Nov 2019 10:26:23 -0700 Subject: [PATCH] Don't maintain replicationfactor of reference tables --- .../master/master_metadata_utility.c | 64 ------------------- .../distributed/sql/citus--9.0-2--9.1-1.sql | 4 ++ src/backend/distributed/utils/node_metadata.c | 12 ---- .../distributed/utils/reference_table_utils.c | 15 ++--- .../distributed/master_metadata_utility.h | 2 - .../multi_remove_node_reference_table.out | 30 ++++----- .../multi_replicate_reference_table.out | 26 ++++---- .../multi_upgrade_reference_table.out | 12 ++-- 8 files changed, 44 insertions(+), 121 deletions(-) diff --git a/src/backend/distributed/master/master_metadata_utility.c b/src/backend/distributed/master/master_metadata_utility.c index 5d0052bb2..dd834f1f3 100644 --- a/src/backend/distributed/master/master_metadata_utility.c +++ b/src/backend/distributed/master/master_metadata_utility.c @@ -1285,70 +1285,6 @@ UpdateShardPlacementState(uint64 placementId, char shardState) } -/* - * UpdateColocationGroupReplicationFactorForReferenceTables updates the - * replicationFactor for the pg_dist_colocation record for reference tables. - * Since we do not cache pg_dist_colocation table, we do not need to invalidate the - * cache after updating replication factor. - */ -void -UpdateColocationGroupReplicationFactorForReferenceTables(int replicationFactor) -{ - Relation pgDistColocation = NULL; - SysScanDesc scanDescriptor = NULL; - ScanKeyData scanKey[1]; - int scanKeyCount = 1; - bool indexOK = false; - HeapTuple heapTuple = NULL; - TupleDesc tupleDescriptor = NULL; - - /* - * All reference tables share a colocation entry with: - * shardCount = 1, replicationFactor = activeWorkerCount, distributiontype = InvalidOid - * Find the record based on distributiontype = InvalidOid, as this uniquely identifies the group. - */ - pgDistColocation = heap_open(DistColocationRelationId(), RowExclusiveLock); - tupleDescriptor = RelationGetDescr(pgDistColocation); - ScanKeyInit(&scanKey[0], Anum_pg_dist_colocation_distributioncolumntype, - BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(InvalidOid)); - - scanDescriptor = systable_beginscan(pgDistColocation, - InvalidOid, indexOK, - NULL, scanKeyCount, scanKey); - - heapTuple = systable_getnext(scanDescriptor); - if (HeapTupleIsValid(heapTuple)) - { - /* after finding the group, update its replication factor */ - /* if it doesn't exist, no worries, it'll be created when needed */ - HeapTuple newHeapTuple = NULL; - Datum values[Natts_pg_dist_colocation]; - bool isnull[Natts_pg_dist_colocation]; - bool replace[Natts_pg_dist_colocation]; - - memset(replace, false, sizeof(replace)); - memset(isnull, false, sizeof(isnull)); - memset(values, 0, sizeof(values)); - - values[Anum_pg_dist_colocation_replicationfactor - 1] = Int32GetDatum( - replicationFactor); - replace[Anum_pg_dist_colocation_replicationfactor - 1] = true; - - newHeapTuple = heap_modify_tuple(heapTuple, tupleDescriptor, values, isnull, - replace); - - CatalogTupleUpdate(pgDistColocation, &newHeapTuple->t_self, newHeapTuple); - - CommandCounterIncrement(); - - heap_freetuple(newHeapTuple); - } - - systable_endscan(scanDescriptor); - heap_close(pgDistColocation, NoLock); -} - - /* * Check that the current user has `mode` permissions on relationId, error out * if not. Superusers always have such permissions. diff --git a/src/backend/distributed/sql/citus--9.0-2--9.1-1.sql b/src/backend/distributed/sql/citus--9.0-2--9.1-1.sql index 4636e0d4b..4da3f4560 100644 --- a/src/backend/distributed/sql/citus--9.0-2--9.1-1.sql +++ b/src/backend/distributed/sql/citus--9.0-2--9.1-1.sql @@ -4,3 +4,7 @@ COMMENT ON COLUMN pg_catalog.pg_dist_node.shouldhaveshards IS #include "udfs/master_set_node_property/9.1-1.sql" #include "udfs/master_drain_node/9.1-1.sql" + +-- we don't maintain replication factor of reference tables anymore and just +-- use -1 instead. +UPDATE pg_dist_colocation SET replicationfactor = -1 WHERE distributioncolumntype = 0; diff --git a/src/backend/distributed/utils/node_metadata.c b/src/backend/distributed/utils/node_metadata.c index d59570b88..5a4b636cd 100644 --- a/src/backend/distributed/utils/node_metadata.c +++ b/src/backend/distributed/utils/node_metadata.c @@ -299,12 +299,6 @@ master_disable_node(PG_FUNCTION_ARGS) SetNodeState(nodeName, nodePort, isActive); - if (WorkerNodeIsPrimary(workerNode)) - { - UpdateColocationGroupReplicationFactorForReferenceTables( - ActivePrimaryNodeCount()); - } - PG_RETURN_VOID(); } @@ -1024,12 +1018,6 @@ RemoveNodeFromCluster(char *nodeName, int32 nodePort) DeleteNodeRow(workerNode->workerName, nodePort); - if (WorkerNodeIsPrimary(workerNode)) - { - UpdateColocationGroupReplicationFactorForReferenceTables( - ActivePrimaryNodeCount()); - } - nodeDeleteCommand = NodeDeleteCommand(workerNode->nodeId); /* make sure we don't have any lingering session lifespan connections */ diff --git a/src/backend/distributed/utils/reference_table_utils.c b/src/backend/distributed/utils/reference_table_utils.c index 536ff5e06..7e9055be5 100644 --- a/src/backend/distributed/utils/reference_table_utils.c +++ b/src/backend/distributed/utils/reference_table_utils.c @@ -130,7 +130,6 @@ ReplicateAllReferenceTablesToNode(char *nodeName, int nodePort) { List *referenceTableList = ReferenceTableOidList(); ListCell *referenceTableCell = NULL; - uint32 workerCount = ActivePrimaryNodeCount(); /* if there is no reference table, we do not need to replicate anything */ if (list_length(referenceTableList) > 0) @@ -181,12 +180,6 @@ ReplicateAllReferenceTablesToNode(char *nodeName, int nodePort) commandList); } } - - /* - * Update replication factor column for colocation group of reference tables - * so that worker count will be equal to replication factor again. - */ - UpdateColocationGroupReplicationFactorForReferenceTables(workerCount); } @@ -389,11 +382,15 @@ uint32 CreateReferenceTableColocationId() { uint32 colocationId = INVALID_COLOCATION_ID; - List *workerNodeList = ActivePrimaryNodeList(ShareLock); int shardCount = 1; - int replicationFactor = list_length(workerNodeList); Oid distributionColumnType = InvalidOid; + /* + * We don't maintain replication factor of reference tables anymore and + * just use -1 instead. We don't use this value in any places. + */ + int replicationFactor = -1; + /* check for existing colocations */ colocationId = ColocationId(shardCount, replicationFactor, distributionColumnType); if (colocationId == INVALID_COLOCATION_ID) diff --git a/src/include/distributed/master_metadata_utility.h b/src/include/distributed/master_metadata_utility.h index bba9ae03f..9d5924b7c 100644 --- a/src/include/distributed/master_metadata_utility.h +++ b/src/include/distributed/master_metadata_utility.h @@ -127,8 +127,6 @@ extern void DeletePartitionRow(Oid distributedRelationId); extern void DeleteShardRow(uint64 shardId); extern void UpdateShardPlacementState(uint64 placementId, char shardState); extern void DeleteShardPlacementRow(uint64 placementId); -extern void UpdateColocationGroupReplicationFactorForReferenceTables(int - replicationFactor); extern void CreateDistributedTable(Oid relationId, Var *distributionColumn, char distributionMethod, char *colocateWithTableName, bool viaDeprecatedAPI); diff --git a/src/test/regress/expected/multi_remove_node_reference_table.out b/src/test/regress/expected/multi_remove_node_reference_table.out index af865d136..a7dceb02d 100644 --- a/src/test/regress/expected/multi_remove_node_reference_table.out +++ b/src/test/regress/expected/multi_remove_node_reference_table.out @@ -142,7 +142,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) @@ -197,7 +197,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port @@ -278,7 +278,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) @@ -336,7 +336,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port @@ -386,7 +386,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) @@ -443,7 +443,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) @@ -501,7 +501,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port @@ -559,7 +559,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) --verify the data is inserted @@ -630,7 +630,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port @@ -687,7 +687,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) @@ -754,7 +754,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) BEGIN; @@ -841,7 +841,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table_schema.table1'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) @@ -898,7 +898,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table_schema.table1'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port @@ -960,7 +960,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port @@ -1017,7 +1017,7 @@ WHERE colocationid IN WHERE logicalrelid = 'remove_node_reference_table'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) \c - - - :worker_1_port diff --git a/src/test/regress/expected/multi_replicate_reference_table.out b/src/test/regress/expected/multi_replicate_reference_table.out index 86baacd65..15ca5ab7e 100644 --- a/src/test/regress/expected/multi_replicate_reference_table.out +++ b/src/test/regress/expected/multi_replicate_reference_table.out @@ -117,7 +117,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_valid'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT 1 FROM master_add_node('localhost', :worker_2_port); @@ -147,7 +147,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_valid'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) -- test add same node twice @@ -171,7 +171,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_valid'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT 1 FROM master_add_node('localhost', :worker_2_port); @@ -200,7 +200,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_valid'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) DROP TABLE replicate_reference_table_valid; @@ -237,7 +237,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_rollback'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) BEGIN; @@ -268,7 +268,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_rollback'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) DROP TABLE replicate_reference_table_rollback; @@ -299,7 +299,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_commit'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) BEGIN; @@ -331,7 +331,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_commit'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) DROP TABLE replicate_reference_table_commit; @@ -381,7 +381,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_reference_one'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -443,7 +443,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_reference_one'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -539,7 +539,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_drop'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) BEGIN; @@ -602,7 +602,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_schema.table1'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 1 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT 1 FROM master_add_node('localhost', :worker_2_port); @@ -632,7 +632,7 @@ WHERE colocationid IN WHERE logicalrelid = 'replicate_reference_table_schema.table1'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) DROP TABLE replicate_reference_table_schema.table1; diff --git a/src/test/regress/expected/multi_upgrade_reference_table.out b/src/test/regress/expected/multi_upgrade_reference_table.out index 1e390acab..ab35cc706 100644 --- a/src/test/regress/expected/multi_upgrade_reference_table.out +++ b/src/test/regress/expected/multi_upgrade_reference_table.out @@ -202,7 +202,7 @@ WHERE colocationid IN WHERE logicalrelid = 'upgrade_reference_table_append'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -315,7 +315,7 @@ WHERE colocationid IN WHERE logicalrelid = 'upgrade_reference_table_one_worker'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -431,7 +431,7 @@ WHERE colocationid IN WHERE logicalrelid = 'upgrade_reference_table_one_unhealthy'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -545,7 +545,7 @@ WHERE colocationid IN WHERE logicalrelid = 'upgrade_reference_table_both_healthy'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -774,7 +774,7 @@ WHERE colocationid IN WHERE logicalrelid = 'upgrade_reference_table_transaction_commit'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT @@ -1023,7 +1023,7 @@ WHERE colocationid IN WHERE logicalrelid = 'upgrade_reference_table_mx'::regclass); colocationid | shardcount | replicationfactor | distributioncolumntype --------------+------------+-------------------+------------------------ - 10004 | 1 | 2 | 0 + 10004 | 1 | -1 | 0 (1 row) SELECT