From 3be665269f008c945de82f6b88ee9cefb3dce383 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Fri, 27 Mar 2020 14:28:50 +0100 Subject: [PATCH] Reintroduce ForceSearchShardPlacementInList (#3664) This was added to silence static analysis errors. It was removed accidentally in #3591. This reintroduces it again. --- .../distributed/master/master_repair_shards.c | 80 +++++++++---------- .../distributed/utils/reference_table_utils.c | 4 +- src/include/distributed/master_protocol.h | 6 +- 3 files changed, 43 insertions(+), 47 deletions(-) diff --git a/src/backend/distributed/master/master_repair_shards.c b/src/backend/distributed/master/master_repair_shards.c index a9275924e..ed6f3f897 100644 --- a/src/backend/distributed/master/master_repair_shards.c +++ b/src/backend/distributed/master/master_repair_shards.c @@ -271,8 +271,6 @@ RepairShardPlacement(int64 shardId, const char *sourceNodeName, int32 sourceNode char relationKind = get_rel_relkind(distributedTableId); char *tableOwner = TableOwner(shardInterval->relationId); - bool missingOk = false; - /* prevent table from being dropped */ LockRelationOid(distributedTableId, AccessShareLock); @@ -369,9 +367,9 @@ RepairShardPlacement(int64 shardId, const char *sourceNodeName, int32 sourceNode /* after successful repair, we update shard state as healthy*/ List *placementList = ShardPlacementList(shardId); - ShardPlacement *placement = SearchShardPlacementInList(placementList, targetNodeName, - targetNodePort, - missingOk); + ShardPlacement *placement = ForceSearchShardPlacementInList(placementList, + targetNodeName, + targetNodePort); UpdateShardPlacementState(placement->placementId, SHARD_STATE_ACTIVE); } @@ -600,23 +598,19 @@ EnsureShardCanBeRepaired(int64 shardId, const char *sourceNodeName, int32 source const char *targetNodeName, int32 targetNodePort) { List *shardPlacementList = ShardPlacementList(shardId); - bool missingSourceOk = false; - bool missingTargetOk = false; - ShardPlacement *sourcePlacement = SearchShardPlacementInList(shardPlacementList, - sourceNodeName, - sourceNodePort, - missingSourceOk); + ShardPlacement *sourcePlacement = ForceSearchShardPlacementInList(shardPlacementList, + sourceNodeName, + sourceNodePort); if (sourcePlacement->shardState != SHARD_STATE_ACTIVE) { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("source placement must be in active state"))); } - ShardPlacement *targetPlacement = SearchShardPlacementInList(shardPlacementList, - targetNodeName, - targetNodePort, - missingTargetOk); + ShardPlacement *targetPlacement = ForceSearchShardPlacementInList(shardPlacementList, + targetNodeName, + targetNodePort); if (targetPlacement->shardState != SHARD_STATE_INACTIVE) { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -634,13 +628,10 @@ EnsureShardCanBeCopied(int64 shardId, const char *sourceNodeName, int32 sourceNo const char *targetNodeName, int32 targetNodePort) { List *shardPlacementList = ShardPlacementList(shardId); - bool missingSourceOk = false; - bool missingTargetOk = true; - ShardPlacement *sourcePlacement = SearchShardPlacementInList(shardPlacementList, - sourceNodeName, - sourceNodePort, - missingSourceOk); + ShardPlacement *sourcePlacement = ForceSearchShardPlacementInList(shardPlacementList, + sourceNodeName, + sourceNodePort); if (sourcePlacement->shardState != SHARD_STATE_ACTIVE) { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -649,8 +640,7 @@ EnsureShardCanBeCopied(int64 shardId, const char *sourceNodeName, int32 sourceNo ShardPlacement *targetPlacement = SearchShardPlacementInList(shardPlacementList, targetNodeName, - targetNodePort, - missingTargetOk); + targetNodePort); if (targetPlacement != NULL) { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -662,42 +652,48 @@ EnsureShardCanBeCopied(int64 shardId, const char *sourceNodeName, int32 sourceNo /* * SearchShardPlacementInList searches a provided list for a shard placement with the - * specified node name and port. If missingOk is set to true, this function returns NULL - * if no such placement exists in the provided list, otherwise it throws an error. + * specified node name and port. This function returns NULL if no such + * placement exists in the provided list. */ ShardPlacement * SearchShardPlacementInList(List *shardPlacementList, const char *nodeName, - uint32 nodePort, bool missingOk) + uint32 nodePort) { - ListCell *shardPlacementCell = NULL; - ShardPlacement *matchingPlacement = NULL; - - foreach(shardPlacementCell, shardPlacementList) + ShardPlacement *shardPlacement = NULL; + foreach_ptr(shardPlacement, shardPlacementList) { - ShardPlacement *shardPlacement = lfirst(shardPlacementCell); - if (strncmp(nodeName, shardPlacement->nodeName, MAX_NODE_LENGTH) == 0 && nodePort == shardPlacement->nodePort) { - matchingPlacement = shardPlacement; - break; + return shardPlacement; } } + return NULL; +} - if (matchingPlacement == NULL) + +/* + * ForceSearchShardPlacementInList searches a provided list for a shard + * placement with the specified node name and port. This function throws an + * error if no such placement exists in the provided list. + * + * This is a separate function (instead of using missingOk), so static analysis + * reasons about NULL returns correctly. + */ +ShardPlacement * +ForceSearchShardPlacementInList(List *shardPlacementList, const char *nodeName, + uint32 nodePort) +{ + ShardPlacement *placement = SearchShardPlacementInList(shardPlacementList, nodeName, + nodePort); + if (placement == NULL) { - if (missingOk) - { - return NULL; - } - ereport(ERROR, (errcode(ERRCODE_DATA_EXCEPTION), errmsg("could not find placement matching \"%s:%d\"", nodeName, nodePort), errhint("Confirm the placement still exists and try again."))); } - - return matchingPlacement; + return placement; } diff --git a/src/backend/distributed/utils/reference_table_utils.c b/src/backend/distributed/utils/reference_table_utils.c index 8f43d5d3a..ebf5350dd 100644 --- a/src/backend/distributed/utils/reference_table_utils.c +++ b/src/backend/distributed/utils/reference_table_utils.c @@ -270,10 +270,8 @@ ReplicateShardToNode(ShardInterval *shardInterval, char *nodeName, int nodePort) CopyShardCommandList(shardInterval, srcNodeName, srcNodePort, includeData); List *shardPlacementList = ShardPlacementList(shardId); - bool missingWorkerOk = true; ShardPlacement *targetPlacement = SearchShardPlacementInList(shardPlacementList, - nodeName, nodePort, - missingWorkerOk); + nodeName, nodePort); char *tableOwner = TableOwner(shardInterval->relationId); /* diff --git a/src/include/distributed/master_protocol.h b/src/include/distributed/master_protocol.h index 8b9978657..b9f5b2671 100644 --- a/src/include/distributed/master_protocol.h +++ b/src/include/distributed/master_protocol.h @@ -165,7 +165,9 @@ extern void CopyShardForeignConstraintCommandListGrouped(ShardInterval *shardInt List ** referenceTableForeignConstraintList); extern ShardPlacement * SearchShardPlacementInList(List *shardPlacementList, - const char *nodeName, uint32 nodePort, - bool missingOk); + const char *nodeName, uint32 nodePort); +extern ShardPlacement * ForceSearchShardPlacementInList(List *shardPlacementList, + const char *nodeName, + uint32 nodePort); #endif /* MASTER_PROTOCOL_H */