zpinto commented on code in PR #2772:
URL: https://github.com/apache/helix/pull/2772#discussion_r1552132848
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -206,113 +205,29 @@ public void addInstance(String clusterName,
InstanceConfig instanceConfig) {
throw new HelixException("Node " + nodeId + " already exists in cluster
" + clusterName);
}
- if (!ALLOWED_INSTANCE_OPERATIONS_FOR_ADD_INSTANCE.contains(
- instanceConfig.getInstanceOperation())) {
+ List<InstanceConfig> matchingLogicalIdInstances =
+ findInstancesMatchingLogicalId(clusterName, instanceConfig);
+ if (matchingLogicalIdInstances.size() > 1) {
throw new HelixException(
- "Instance can only be added if InstanceOperation is set to one of" +
"the following: "
- + ALLOWED_INSTANCE_OPERATIONS_FOR_ADD_INSTANCE + " This
instance: " + nodeId
- + " has InstanceOperation set to " +
instanceConfig.getInstanceOperation());
+ "There are already more than one instance with the same logicalId in
the cluster: "
+ +
matchingLogicalIdInstances.stream().map(InstanceConfig::getInstanceName)
+ .collect(Collectors.joining(", "))
+ + " Please make sure there is at most 2 instance with the same
logicalId in the cluster.");
}
- // Get the topology key used to determine the logicalId of a node.
- ClusterConfig clusterConfig =
_configAccessor.getClusterConfig(clusterName);
- ClusterTopologyConfig clusterTopologyConfig =
- ClusterTopologyConfig.createFromClusterConfig(clusterConfig);
- String logicalIdKey = clusterTopologyConfig.getEndNodeType();
- String faultZoneKey = clusterTopologyConfig.getFaultZoneType();
- String toAddInstanceLogicalId = instanceConfig.getLogicalId(logicalIdKey);
-
- HelixConfigScope instanceConfigScope =
- new
HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.PARTICIPANT,
- clusterName).build();
- List<String> existingInstanceIds = getConfigKeys(instanceConfigScope);
- List<InstanceConfig> foundInstanceConfigsWithMatchingLogicalId =
- existingInstanceIds.parallelStream()
- .map(existingInstanceId -> getInstanceConfig(clusterName,
existingInstanceId)).filter(
- existingInstanceConfig ->
existingInstanceConfig.getLogicalId(logicalIdKey)
-
.equals(toAddInstanceLogicalId)).collect(Collectors.toList());
-
- if (foundInstanceConfigsWithMatchingLogicalId.size() >= 2) {
- // If the length is 2, we cannot add an instance with the same logicalId
as an existing instance
- // regardless of InstanceOperation.
- throw new HelixException(
- "There can only be 2 instances with the same logicalId in a cluster.
"
- + "Existing instances: " +
foundInstanceConfigsWithMatchingLogicalId.get(0)
- .getInstanceName() + " and " +
foundInstanceConfigsWithMatchingLogicalId.get(1)
- .getInstanceName() + " already have the same logicalId: " +
toAddInstanceLogicalId
- + "; therefore, " + nodeId + " cannot be added to the cluster.");
- } else if (foundInstanceConfigsWithMatchingLogicalId.size() == 1) {
- // If there is only one instance with the same logicalId,
- // we can infer that the intended behaviour is to SWAP_IN or EVACUATE +
ADD.
- if
(foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceOperation()
- .equals(InstanceConstants.InstanceOperation.SWAP_OUT.name())) {
- // If the existing instance with the same logicalId has SWAP_OUT
InstanceOperation
-
- // If the InstanceOperation is unset, we will set it to SWAP_IN.
- if (!instanceConfig.getInstanceOperation()
- .equals(InstanceConstants.InstanceOperation.SWAP_IN.name())) {
-
instanceConfig.setInstanceOperation(InstanceConstants.InstanceOperation.SWAP_IN);
- }
-
- // If the existing instance with the same logicalId is not in the same
FAULT_ZONE as this instance, we cannot
- // add this instance.
- if (!foundInstanceConfigsWithMatchingLogicalId.get(0).getDomainAsMap()
- .containsKey(faultZoneKey) ||
!instanceConfig.getDomainAsMap().containsKey(faultZoneKey)
- ||
!foundInstanceConfigsWithMatchingLogicalId.get(0).getDomainAsMap().get(faultZoneKey)
- .equals(instanceConfig.getDomainAsMap().get(faultZoneKey))) {
- throw new HelixException(
- "Instance can only be added if the SWAP_OUT instance sharing the
same logicalId is in the same FAULT_ZONE"
- + " as this instance. " + "Existing instance: "
- +
foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceName()
- + " has FAULT_ZONE_TYPE: " +
foundInstanceConfigsWithMatchingLogicalId.get(0)
- .getDomainAsMap().get(faultZoneKey) + " and this instance: "
+ nodeId
- + " has FAULT_ZONE_TYPE: " +
instanceConfig.getDomainAsMap().get(faultZoneKey));
- }
-
- Map<String, Integer> foundInstanceCapacityMap =
-
foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceCapacityMap().isEmpty()
- ? clusterConfig.getDefaultInstanceCapacityMap()
- :
foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceCapacityMap();
- Map<String, Integer> instanceCapacityMap =
instanceConfig.getInstanceCapacityMap().isEmpty()
- ? clusterConfig.getDefaultInstanceCapacityMap()
- : instanceConfig.getInstanceCapacityMap();
- // If the instance does not have the same capacity, we cannot add this
instance.
- if (!new EqualsBuilder().append(foundInstanceCapacityMap,
instanceCapacityMap).isEquals()) {
- throw new HelixException(
- "Instance can only be added if the SWAP_OUT instance sharing the
same logicalId has the same capacity"
- + " as this instance. " + "Existing instance: "
- +
foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceName()
- + " has capacity: " + foundInstanceCapacityMap + " and this
instance: " + nodeId
- + " has capacity: " + instanceCapacityMap);
- }
- } else if
(foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceOperation()
- .equals(InstanceConstants.InstanceOperation.EVACUATE.name())) {
- // No need to check anything on the new node, the old node will be
evacuated and the new node
- // will be added.
- } else {
- // If the instanceConfig.getInstanceEnabled() is true and the existing
instance with the same logicalId
- // does not have InstanceOperation set to one of the above, we cannot
add this instance.
- throw new HelixException(
- "Instance can only be added if the exising instance sharing the
same logicalId"
- + " has InstanceOperation set to "
- + InstanceConstants.InstanceOperation.SWAP_OUT.name()
- + " and this instance has InstanceOperation set to "
- + InstanceConstants.InstanceOperation.SWAP_IN.name()
- + " or the existing instance sharing the same logicalId has
Instance Operation set to "
- + InstanceConstants.InstanceOperation.EVACUATE.name()
- + " and this instance has InstanceOperation unset. Existing
instance: "
- +
foundInstanceConfigsWithMatchingLogicalId.get(0).getInstanceName()
- + " has InstanceOperation: " +
foundInstanceConfigsWithMatchingLogicalId.get(0)
- .getInstanceOperation());
- }
- } else if (!instanceConfig.getInstanceOperation().isEmpty()) {
- // If there are no instances with the same logicalId, we can only add
this instance if InstanceOperation
- // is unset because it is a new instance.
- throw new HelixException(
- "There is no instance with logicalId: " + toAddInstanceLogicalId + "
in cluster: "
- + clusterName + "; therefore, " + nodeId
- + " cannot join cluster with InstanceOperation set to "
- + instanceConfig.getInstanceOperation() + ".");
+ InstanceConstants.InstanceOperation attemptedInstanceOperation =
+ instanceConfig.getInstanceOperation();
+ try {
+ validateInstanceOperationTransition(instanceConfig,
+ !matchingLogicalIdInstances.isEmpty() ?
matchingLogicalIdInstances.get(0) : null,
+ InstanceConstants.InstanceOperation.UNKNOWN,
+ attemptedInstanceOperation, clusterName);
+ } catch (HelixException e) {
+
instanceConfig.setInstanceOperation(InstanceConstants.InstanceOperation.UNKNOWN);
+ logger.error("Failed to add instance " +
instanceConfig.getInstanceName() + " to cluster "
+ + clusterName + " with instance operation " +
attemptedInstanceOperation
+ + ". Setting INSTANCE_OPERATION to " +
instanceConfig.getInstanceOperation()
+ + " instead.", e);
Review Comment:
This will only be called by the participant or externally one time so the
logs should not keep being printed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
