From f4e8af2c224613694999021cfb22001160a86a5d Mon Sep 17 00:00:00 2001 From: Hanefi Onaldi Date: Mon, 21 Feb 2022 22:29:00 +0300 Subject: [PATCH] Do not acquire locks on node metadata explicitly --- src/backend/distributed/commands/extension.c | 39 ------------------- src/backend/distributed/commands/role.c | 7 ---- src/backend/distributed/commands/type.c | 10 ----- .../distributed/metadata/node_metadata.c | 9 ----- 4 files changed, 65 deletions(-) diff --git a/src/backend/distributed/commands/extension.c b/src/backend/distributed/commands/extension.c index dc1363a65..fac8a783a 100644 --- a/src/backend/distributed/commands/extension.c +++ b/src/backend/distributed/commands/extension.c @@ -148,16 +148,6 @@ PostprocessCreateExtensionStmt(Node *node, const char *queryString) /* extension management can only be done via coordinator node */ EnsureCoordinator(); - /* - * Make sure that no new nodes are added after this point until the end of the - * transaction by taking a RowShareLock on pg_dist_node, which conflicts with the - * ExclusiveLock taken by citus_add_node. - * This guarantees that all active nodes will have the extension, because they will - * either get it now, or get it in citus_add_node after this transaction finishes and - * the pg_dist_object record becomes visible. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - /* * Make sure that the current transaction is already in sequential mode, * or can still safely be put in sequential mode @@ -259,16 +249,6 @@ PreprocessDropExtensionStmt(Node *node, const char *queryString, /* extension management can only be done via coordinator node */ EnsureCoordinator(); - /* - * Make sure that no new nodes are added after this point until the end of the - * transaction by taking a RowShareLock on pg_dist_node, which conflicts with the - * ExclusiveLock taken by citus_add_node. - * This guarantees that all active nodes will drop the extension, because they will - * either get it now, or get it in citus_add_node after this transaction finishes and - * the pg_dist_object record becomes visible. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - /* * Make sure that the current transaction is already in sequential mode, * or can still safely be put in sequential mode @@ -395,15 +375,6 @@ PreprocessAlterExtensionSchemaStmt(Node *node, const char *queryString, /* extension management can only be done via coordinator node */ EnsureCoordinator(); - /* - * Make sure that no new nodes are added after this point until the end of the - * transaction by taking a RowShareLock on pg_dist_node, which conflicts with the - * ExclusiveLock taken by citus_add_node. - * This guarantees that all active nodes will update the extension schema after - * this transaction finishes and the pg_dist_object record becomes visible. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - /* * Make sure that the current transaction is already in sequential mode, * or can still safely be put in sequential mode @@ -463,16 +434,6 @@ PreprocessAlterExtensionUpdateStmt(Node *node, const char *queryString, /* extension management can only be done via coordinator node */ EnsureCoordinator(); - /* - * Make sure that no new nodes are added after this point until the end of the - * transaction by taking a RowShareLock on pg_dist_node, which conflicts with the - * ExclusiveLock taken by citus_add_node. - * This guarantees that all active nodes will update the extension version, because - * they will either get it now, or get it in citus_add_node after this transaction - * finishes and the pg_dist_object record becomes visible. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - /* * Make sure that the current transaction is already in sequential mode, * or can still safely be put in sequential mode diff --git a/src/backend/distributed/commands/role.c b/src/backend/distributed/commands/role.c index c84899453..608dc0060 100644 --- a/src/backend/distributed/commands/role.c +++ b/src/backend/distributed/commands/role.c @@ -140,13 +140,6 @@ PostprocessAlterRoleStmt(Node *node, const char *queryString) AlterRoleStmt *stmt = castNode(AlterRoleStmt, node); - /* - * Make sure that no new nodes are added after this point until the end of the - * transaction by taking a RowShareLock on pg_dist_node, which conflicts with the - * ExclusiveLock taken by citus_add_node. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - DefElem *option = NULL; foreach_ptr(option, stmt->options) { diff --git a/src/backend/distributed/commands/type.c b/src/backend/distributed/commands/type.c index eb59e8522..47dd0c307 100644 --- a/src/backend/distributed/commands/type.c +++ b/src/backend/distributed/commands/type.c @@ -129,16 +129,6 @@ PreprocessCompositeTypeStmt(Node *node, const char *queryString, */ EnsureCoordinator(); - /* - * Make sure that no new nodes are added after this point until the end of the - * transaction by taking a RowShareLock on pg_dist_node, which conflicts with the - * ExclusiveLock taken by citus_add_node. - * This guarantees that all active nodes will have the object, because they will - * either get it now, or get it in citus_add_node after this transaction finishes and - * the pg_dist_object record becomes visible. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - /* fully qualify before lookup and later deparsing */ QualifyTreeNode(node); diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index 706f000cb..967d6bef3 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -188,9 +188,6 @@ citus_set_coordinator_host(PG_FUNCTION_ARGS) Name nodeClusterName = PG_GETARG_NAME(3); nodeMetadata.nodeCluster = NameStr(*nodeClusterName); - /* prevent concurrent modification */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - bool isCoordinatorInMetadata = false; WorkerNode *coordinatorNode = PrimaryNodeForGroup(COORDINATOR_GROUP_ID, &isCoordinatorInMetadata); @@ -1780,12 +1777,6 @@ AddNodeMetadata(char *nodeName, int32 nodePort, *nodeAlreadyExists = false; - /* - * Prevent / wait for concurrent modification before checking whether - * the worker already exists in pg_dist_node. - */ - LockRelationOid(DistNodeRelationId(), RowShareLock); - WorkerNode *workerNode = FindWorkerNodeAnyCluster(nodeName, nodePort); if (workerNode != NULL) {