From 64c65b6234bfa2600c2a56f7ca92771e262995b2 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Sun, 6 Mar 2022 18:19:40 +0100 Subject: [PATCH] Add a GUC to require coordinator in the metadata --- .../citus_add_local_table_to_metadata.c | 3 ++ .../commands/create_distributed_table.c | 6 ++++ .../distributed/metadata/node_metadata.c | 35 +++++++++++++++++++ src/backend/distributed/shared_library_init.c | 15 ++++++++ src/include/distributed/worker_manager.h | 2 ++ .../expected/multi_cluster_management.out | 7 ++++ src/test/regress/pg_regress_multi.pl | 3 ++ .../regress/sql/multi_cluster_management.sql | 6 ++++ 8 files changed, 77 insertions(+) diff --git a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c index e447a2be1..daf812b28 100644 --- a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c +++ b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c @@ -201,6 +201,9 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys, bool autoConve /* enable citus_add_local_table_to_metadata on an empty node */ InsertCoordinatorIfClusterEmpty(); + /* make sure the coordinator is in the metadata */ + EnsureCoordinatorInMetadata(); + /* * Creating Citus local tables relies on functions that accesses * shards locally (e.g., ExecuteAndLogUtilityCommand()). As long as diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index c639d836c..2e203e775 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -218,6 +218,9 @@ create_distributed_table(PG_FUNCTION_ARGS) /* enable create_distributed_table on an empty node */ InsertCoordinatorIfClusterEmpty(); + /* make sure the coordinator is in the metadata */ + EnsureCoordinatorInMetadata(); + /* * Lock target relation with an exclusive lock - there's no way to make * sense of this table until we've committed, and we don't want multiple @@ -273,6 +276,9 @@ create_reference_table(PG_FUNCTION_ARGS) /* enable create_reference_table on an empty node */ InsertCoordinatorIfClusterEmpty(); + /* make sure the coordinator is in the metadata */ + EnsureCoordinatorInMetadata(); + /* * Lock target relation with an exclusive lock - there's no way to make * sense of this table until we've committed, and we don't want multiple diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index 6f80558e8..82d9404dc 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -77,6 +77,13 @@ bool ReplicateReferenceTablesOnActivate = true; /* did current transaction modify pg_dist_node? */ bool TransactionModifiedNodeMetadata = false; +/* + * IsCoordinatorInMetadataRequired is a GUC that specifies whether operations + * like create_distributed_table should fail if the coordinator is not in the + * metadata. + */ +bool IsCoordinatorInMetadataRequired = true; + bool EnableMetadataSync = true; typedef struct NodeMetadata @@ -2356,6 +2363,34 @@ EnsureCoordinator(void) } +/* + * EnsureCoordinatorInMetadata throws an error if the coordinator is not + * in the metadata. + */ +void +EnsureCoordinatorInMetadata(void) +{ + if (!IsCoordinatorInMetadataRequired) + { + return; + } + + bool isCoordinatorInMetadata = false; + + PrimaryNodeForGroup(COORDINATOR_GROUP_ID, &isCoordinatorInMetadata); + + if (!isCoordinatorInMetadata) + { + ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("coordinator is not yet added to the Citus node metadata"), + errdetail("Worker nodes need to be able to connect to the " + "coordinator to transfer data."), + errhint("Use SELECT citus_set_coordinator_host('') " + "to configure the coordinator hostname"))); + } +} + + /* * InsertCoordinatorIfClusterEmpty can be used to ensure Citus tables can be * created even on a node that has just performed CREATE EXTENSION citus; diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index 4694e1d50..ba58f7625 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -1642,6 +1642,21 @@ RegisterCitusConfigVariables(void) GUC_NO_SHOW_ALL, WarnIfReplicationModelIsSet, NULL, NULL); + DefineCustomBoolVariable( + "citus.require_coordinator_in_metadata", + gettext_noop("Sets whether Citus requires that the coordinator is in the " + "metadata"), + gettext_noop("Various Citus features depend on the coordinator being in the " + "metadata. By default, we check whether this is the case when " + "creating a Citus table. You can circumvent this check by " + "disabling this setting."), + &IsCoordinatorInMetadataRequired, + true, + PGC_SUSET, + GUC_NO_SHOW_ALL, + NULL, NULL, NULL); + + DefineCustomBoolVariable( "citus.running_under_isolation_test", gettext_noop( diff --git a/src/include/distributed/worker_manager.h b/src/include/distributed/worker_manager.h index 27de1d464..207ffd5ed 100644 --- a/src/include/distributed/worker_manager.h +++ b/src/include/distributed/worker_manager.h @@ -62,6 +62,7 @@ extern int MaxWorkerNodesTracked; extern char *WorkerListFileName; extern char *CurrentCluster; extern bool ReplicateReferenceTablesOnActivate; +extern bool IsCoordinatorInMetadataRequired; extern int ActivateNode(char *nodeName, int nodePort); @@ -89,6 +90,7 @@ extern WorkerNode * FindWorkerNodeAnyCluster(const char *nodeName, int32 nodePor extern WorkerNode * FindNodeWithNodeId(int nodeId, bool missingOk); extern List * ReadDistNode(bool includeNodesFromOtherClusters); extern void EnsureCoordinator(void); +void EnsureCoordinatorInMetadata(void); extern void InsertCoordinatorIfClusterEmpty(void); extern uint32 GroupForNode(char *nodeName, int32 nodePort); extern WorkerNode * PrimaryNodeForGroup(int32 groupId, bool *groupContainsNodes); diff --git a/src/test/regress/expected/multi_cluster_management.out b/src/test/regress/expected/multi_cluster_management.out index 0903e0e36..03ee919b1 100644 --- a/src/test/regress/expected/multi_cluster_management.out +++ b/src/test/regress/expected/multi_cluster_management.out @@ -115,6 +115,13 @@ SELECT 1 FROM citus_activate_node('localhost', :worker_2_port); (1 row) CREATE TABLE cluster_management_test (col_1 text, col_2 int); +-- coordinator was not yet added to the metadata yet, should fail +SET citus.require_coordinator_in_metadata TO on; +SELECT create_distributed_table('cluster_management_test', 'col_1', 'hash'); +ERROR: coordinator is not yet added to the Citus node metadata +DETAIL: Worker nodes need to be able to connect to the coordinator to transfer data. +HINT: Use SELECT citus_set_coordinator_host('') to configure the coordinator hostname +RESET citus.require_coordinator_in_metadata; SELECT create_distributed_table('cluster_management_test', 'col_1', 'hash'); create_distributed_table --------------------------------------------------------------------- diff --git a/src/test/regress/pg_regress_multi.pl b/src/test/regress/pg_regress_multi.pl index ebbb15371..24348d609 100755 --- a/src/test/regress/pg_regress_multi.pl +++ b/src/test/regress/pg_regress_multi.pl @@ -465,6 +465,9 @@ push(@pgOptions, "citus.node_connection_timeout=${connectionTimeout}"); push(@pgOptions, "citus.explain_analyze_sort_method='taskId'"); push(@pgOptions, "citus.enable_manual_changes_to_shards=on"); +# We currently have too many tests that omit coordinator from metadata +push(@pgOptions, "citus.require_coordinator_in_metadata=off"); + # Some tests look at shards in pg_class, make sure we can usually see them: push(@pgOptions, "citus.hide_shards_from_app_name_prefixes='psql,pg_dump'"); diff --git a/src/test/regress/sql/multi_cluster_management.sql b/src/test/regress/sql/multi_cluster_management.sql index b24c79232..9181d7a62 100644 --- a/src/test/regress/sql/multi_cluster_management.sql +++ b/src/test/regress/sql/multi_cluster_management.sql @@ -52,6 +52,12 @@ ALTER SEQUENCE pg_catalog.pg_dist_colocationid_seq RESTART 1390000; SELECT 1 FROM citus_activate_node('localhost', :worker_2_port); CREATE TABLE cluster_management_test (col_1 text, col_2 int); + +-- coordinator was not yet added to the metadata yet, should fail +SET citus.require_coordinator_in_metadata TO on; +SELECT create_distributed_table('cluster_management_test', 'col_1', 'hash'); +RESET citus.require_coordinator_in_metadata; + SELECT create_distributed_table('cluster_management_test', 'col_1', 'hash'); -- see that there are some active placements in the candidate node