From 010a2a408e309304cc55e8ee24bc525d133c4cc9 Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Mon, 30 May 2022 12:22:09 +0200 Subject: [PATCH 1/3] Avoid assertion failure on citus_add_node --- src/backend/distributed/metadata/node_metadata.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index c19148317..d0a166f14 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -746,8 +746,6 @@ PgDistTableMetadataSyncCommandList(void) metadataSnapshotCommandList = list_concat(metadataSnapshotCommandList, colocationGroupSyncCommandList); - /* As the last step, propagate the pg_dist_object entities */ - Assert(ShouldPropagate()); List *distributedObjectSyncCommandList = DistributedObjectMetadataSyncCommandList(); metadataSnapshotCommandList = list_concat(metadataSnapshotCommandList, distributedObjectSyncCommandList); @@ -924,8 +922,6 @@ SyncPgDistTableMetadataToNodeList(List *nodeList) /* send commands to new workers, the current user should be a superuser */ Assert(superuser()); - List *syncPgDistMetadataCommandList = PgDistTableMetadataSyncCommandList(); - List *nodesWithMetadata = NIL; WorkerNode *workerNode = NULL; foreach_ptr(workerNode, nodeList) @@ -936,7 +932,12 @@ SyncPgDistTableMetadataToNodeList(List *nodeList) } } + if (nodesWithMetadata == NIL) + { + return; + } + List *syncPgDistMetadataCommandList = PgDistTableMetadataSyncCommandList(); SendMetadataCommandListToWorkerListInCoordinatedTransaction( nodesWithMetadata, CurrentUserName(), From 7157152f6c19809b7ae16ee10a7c6c9fdee079ce Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Mon, 30 May 2022 12:25:19 +0200 Subject: [PATCH 2/3] Do not send metadata changes during add node if citus.enable_metadata_sync is set to false --- .../distributed/metadata/node_metadata.c | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index d0a166f14..23d4d3f4d 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -1899,12 +1899,15 @@ RemoveNodeFromCluster(char *nodeName, int32 nodePort) RemoveOldShardPlacementForNodeGroup(workerNode->groupId); - char *nodeDeleteCommand = NodeDeleteCommand(workerNode->nodeId); - /* make sure we don't have any lingering session lifespan connections */ CloseNodeConnectionsAfterTransaction(workerNode->workerName, nodePort); - SendCommandToWorkersWithMetadata(nodeDeleteCommand); + if (EnableMetadataSync) + { + char *nodeDeleteCommand = NodeDeleteCommand(workerNode->nodeId); + + SendCommandToWorkersWithMetadata(nodeDeleteCommand); + } } @@ -2155,18 +2158,21 @@ AddNodeMetadata(char *nodeName, int32 nodePort, workerNode = FindWorkerNodeAnyCluster(nodeName, nodePort); - /* send the delete command to all primary nodes with metadata */ - char *nodeDeleteCommand = NodeDeleteCommand(workerNode->nodeId); - SendCommandToWorkersWithMetadata(nodeDeleteCommand); - - /* finally prepare the insert command and send it to all primary nodes */ - uint32 primariesWithMetadata = CountPrimariesWithMetadata(); - if (primariesWithMetadata != 0) + if (EnableMetadataSync) { - List *workerNodeList = list_make1(workerNode); - char *nodeInsertCommand = NodeListInsertCommand(workerNodeList); + /* send the delete command to all primary nodes with metadata */ + char *nodeDeleteCommand = NodeDeleteCommand(workerNode->nodeId); + SendCommandToWorkersWithMetadata(nodeDeleteCommand); - SendCommandToWorkersWithMetadata(nodeInsertCommand); + /* finally prepare the insert command and send it to all primary nodes */ + uint32 primariesWithMetadata = CountPrimariesWithMetadata(); + if (primariesWithMetadata != 0) + { + List *workerNodeList = list_make1(workerNode); + char *nodeInsertCommand = NodeListInsertCommand(workerNodeList); + + SendCommandToWorkersWithMetadata(nodeInsertCommand); + } } return workerNode->nodeId; @@ -2185,11 +2191,13 @@ SetWorkerColumn(WorkerNode *workerNode, int columnIndex, Datum value) { workerNode = SetWorkerColumnLocalOnly(workerNode, columnIndex, value); - char *metadataSyncCommand = GetMetadataSyncCommandToSetNodeColumn(workerNode, - columnIndex, - value); + if (EnableMetadataSync) + { + char *metadataSyncCommand = + GetMetadataSyncCommandToSetNodeColumn(workerNode, columnIndex, value); - SendCommandToWorkersWithMetadata(metadataSyncCommand); + SendCommandToWorkersWithMetadata(metadataSyncCommand); + } return workerNode; } From 89c1ccb7a5592c705122f01922baadc5aaecb60f Mon Sep 17 00:00:00 2001 From: Onder Kalaci Date: Mon, 30 May 2022 12:45:00 +0200 Subject: [PATCH 3/3] Show that no metadata is sent when disabled --- .../expected/multi_cluster_management.out | 46 +++++++++++++++++++ .../regress/sql/multi_cluster_management.sql | 20 ++++++++ 2 files changed, 66 insertions(+) diff --git a/src/test/regress/expected/multi_cluster_management.out b/src/test/regress/expected/multi_cluster_management.out index fce2bcbd2..cb3fe3a5d 100644 --- a/src/test/regress/expected/multi_cluster_management.out +++ b/src/test/regress/expected/multi_cluster_management.out @@ -1190,6 +1190,52 @@ WHERE logicalrelid = 'test_dist_non_colocated'::regclass GROUP BY nodeport ORDER SELECT * from master_set_node_property('localhost', :worker_2_port, 'bogusproperty', false); ERROR: only the 'shouldhaveshards' property can be set using this function +SELECT nextval('pg_catalog.pg_dist_groupid_seq') AS last_group_id_cls \gset +SELECT nextval('pg_catalog.pg_dist_node_nodeid_seq') AS last_node_id_cls \gset +BEGIN; + -- show that we do not send any metadata to any nodes if not enabled + SET LOCAL citus.log_remote_commands TO ON; + SET LOCAL citus.grep_remote_commands TO '%pg_dist%'; + SET citus.enable_metadata_sync TO OFF; + SELECT start_metadata_sync_to_all_nodes(); + start_metadata_sync_to_all_nodes +--------------------------------------------------------------------- + t +(1 row) + + DROP TABLE test_dist, test_ref, test_dist_colocated, test_dist_non_colocated; + SELECT 1 FROM citus_remove_node('localhost', :worker_1_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + + SELECT 1 FROM citus_remove_node('localhost', :worker_2_port); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + + SELECT 1 FROM citus_add_node('localhost', :worker_1_port); +NOTICE: issuing SELECT metadata ->> 'server_id' AS server_id FROM pg_dist_node_metadata +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + + SELECT 1 FROM citus_add_node('localhost', :worker_2_port); +NOTICE: issuing SELECT metadata ->> 'server_id' AS server_id FROM pg_dist_node_metadata +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +ROLLBACK; +-- keep the rest of the tests inact that depends node/group ids +ALTER SEQUENCE pg_catalog.pg_dist_groupid_seq RESTART :last_group_id_cls; +ALTER SEQUENCE pg_catalog.pg_dist_node_nodeid_seq RESTART :last_node_id_cls; DROP TABLE test_dist, test_ref, test_dist_colocated, test_dist_non_colocated; BEGIN; SELECT start_metadata_sync_to_all_nodes(); diff --git a/src/test/regress/sql/multi_cluster_management.sql b/src/test/regress/sql/multi_cluster_management.sql index 4162ad7c6..6d9b9c589 100644 --- a/src/test/regress/sql/multi_cluster_management.sql +++ b/src/test/regress/sql/multi_cluster_management.sql @@ -482,6 +482,26 @@ WHERE logicalrelid = 'test_dist_non_colocated'::regclass GROUP BY nodeport ORDER SELECT * from master_set_node_property('localhost', :worker_2_port, 'bogusproperty', false); +SELECT nextval('pg_catalog.pg_dist_groupid_seq') AS last_group_id_cls \gset +SELECT nextval('pg_catalog.pg_dist_node_nodeid_seq') AS last_node_id_cls \gset + +BEGIN; + -- show that we do not send any metadata to any nodes if not enabled + SET LOCAL citus.log_remote_commands TO ON; + SET LOCAL citus.grep_remote_commands TO '%pg_dist%'; + SET citus.enable_metadata_sync TO OFF; + SELECT start_metadata_sync_to_all_nodes(); + DROP TABLE test_dist, test_ref, test_dist_colocated, test_dist_non_colocated; + SELECT 1 FROM citus_remove_node('localhost', :worker_1_port); + SELECT 1 FROM citus_remove_node('localhost', :worker_2_port); + SELECT 1 FROM citus_add_node('localhost', :worker_1_port); + SELECT 1 FROM citus_add_node('localhost', :worker_2_port); +ROLLBACK; + +-- keep the rest of the tests inact that depends node/group ids +ALTER SEQUENCE pg_catalog.pg_dist_groupid_seq RESTART :last_group_id_cls; +ALTER SEQUENCE pg_catalog.pg_dist_node_nodeid_seq RESTART :last_node_id_cls; + DROP TABLE test_dist, test_ref, test_dist_colocated, test_dist_non_colocated; BEGIN;