zpinto commented on code in PR #2772:
URL: https://github.com/apache/helix/pull/2772#discussion_r1552149327
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -509,62 +413,107 @@ public void enableInstance(String clusterName,
List<String> instances, boolean e
//enableInstance(clusterName, instances, enabled, null, null);
}
+ private void validateInstanceOperationTransition(InstanceConfig
instanceConfig,
+ InstanceConfig matchingLogicalIdInstance,
+ InstanceConstants.InstanceOperation currentOperation,
+ InstanceConstants.InstanceOperation targetOperation,
+ String clusterName) {
+ boolean targetStateEnableOrDisable =
+ targetOperation.equals(InstanceConstants.InstanceOperation.ENABLE)
+ ||
targetOperation.equals(InstanceConstants.InstanceOperation.DISABLE);
+ switch (currentOperation) {
+ case ENABLE:
+ case DISABLE:
+ // ENABLE or DISABLE can be set to ENABLE, DISABLE, or EVACUATE at any
time.
+ if (ImmutableSet.of(InstanceConstants.InstanceOperation.ENABLE,
+ InstanceConstants.InstanceOperation.DISABLE,
+
InstanceConstants.InstanceOperation.EVACUATE).contains(targetOperation)) {
+ return;
+ }
+ case SWAP_IN:
+ // We can only ENABLE or DISABLE a SWAP_IN instance if there is an
instance with matching logicalId
+ // with an InstanceOperation set to UNKNOWN.
+ if ((targetStateEnableOrDisable && (matchingLogicalIdInstance == null
+ || matchingLogicalIdInstance.getInstanceOperation()
+ .equals(InstanceConstants.InstanceOperation.UNKNOWN))) ||
targetOperation.equals(
+ InstanceConstants.InstanceOperation.UNKNOWN)) {
+ return;
+ }
+ case EVACUATE:
+ // EVACUATE can only be set to ENABLE or DISABLE when there is no
instance with the same
+ // logicalId in the cluster.
+ if ((targetStateEnableOrDisable && matchingLogicalIdInstance == null)
+ ||
targetOperation.equals(InstanceConstants.InstanceOperation.UNKNOWN)) {
+ return;
+ }
+ case UNKNOWN:
+ // UNKNOWN can be set to ENABLE or DISABLE when there is no instance
with the same logicalId in the cluster
+ // or the instance with the same logicalId in the cluster has
InstanceOperation set to EVACUATE.
+ // UNKNOWN can be set to SWAP_IN when there is an instance with the
same logicalId in the cluster set to ENABLE,
+ // or DISABLE.
+ if ((targetStateEnableOrDisable && (matchingLogicalIdInstance == null
+ || matchingLogicalIdInstance.getInstanceOperation()
+ .equals(InstanceConstants.InstanceOperation.EVACUATE)))) {
+ return;
+ } else if
(targetOperation.equals(InstanceConstants.InstanceOperation.SWAP_IN)
+ && matchingLogicalIdInstance != null && !ImmutableSet.of(
+ InstanceConstants.InstanceOperation.UNKNOWN,
+ InstanceConstants.InstanceOperation.EVACUATE)
+ .contains(matchingLogicalIdInstance.getInstanceOperation())) {
+ // Get the topology key used to determine the logicalId of a node.
+ ClusterConfig clusterConfig =
_configAccessor.getClusterConfig(clusterName);
+ ClusterTopologyConfig clusterTopologyConfig =
+ ClusterTopologyConfig.createFromClusterConfig(clusterConfig);
+ String faultZoneKey = clusterTopologyConfig.getFaultZoneType();
+ // If the existing instance with the same logicalId is not in the
same FAULT_ZONE as this instance, we cannot
+ // add this instance.
+ if
(!matchingLogicalIdInstance.getDomainAsMap().containsKey(faultZoneKey)
Review Comment:
If the user is going to add nodes with same logicalId but in different
zones, they should not be using swap. They should use Evac and Add so that the
the assignment does not get mirrored on the new node and then potentially
dropped right after the swap is marked complete. We can discuss more offline :)
--
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]
