desaikomal commented on code in PR #2792:
URL: https://github.com/apache/helix/pull/2792#discussion_r1567959515
##########
helix-core/src/main/java/org/apache/helix/HelixAdmin.java:
##########
@@ -415,6 +415,17 @@ void manuallyEnableMaintenanceMode(String clusterName,
boolean enabled, String r
*/
ClusterManagementMode getClusterManagementMode(String clusterName);
+ /**
+ * Set a list of partitions for an instance to ERROR from any current state.
+ * The partitions could be in any state and setPartitionToError will bring
them to ERROR
+ * from their current state. A CURRENT_STATE to ERROR state transition is
required for this.
+ * @param clusterName
+ * @param instanceName
+ * @param resourceName
+ * @param partitionNames
+ */
+ void setPartitionToError(String clusterName, String instanceName, String
resourceName, List<String> partitionNames);
Review Comment:
is it private method?
nit: since it is taking list of partitionNames, should be called
`setPartitionsToError` ?
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -1085,6 +1085,139 @@ public ClusterManagementMode
getClusterManagementMode(String clusterName) {
: new ClusterManagementMode(status.getManagementMode(),
status.getManagementModeStatus());
}
+ private enum SetPartitionToErrorFailureReason {
+ INSTANCE_NOT_ALIVE("%s is not alive in cluster %s"),
+ INSTANCE_NON_EXISTENT("%s does not exist in cluster %s"),
+ RESOURCE_NON_EXISTENT("resource %s is not added to cluster %s"),
+ PARTITION_NON_EXISTENT("not all %s exist in cluster %s"),
+ PARTITION_ALREADY_IN_ERROR("%s is NOT found in cluster %s or is already in
ERROR state"),
+ STATE_MODEL_NON_EXISTENT("%s is NOT found in cluster %s");
+
+ private String message;
+
+ SetPartitionToErrorFailureReason(String message) {
+ this.message = message;
+ }
+
+ public String getMessage(String resourceName, List<String> partitionNames,
String instanceName,
+ String errorStateEntity, String clusterName) {
+ return String.format("Can't set to Error State for %s.%s on %s, because
" + message, resourceName,
+ partitionNames, instanceName, errorStateEntity, clusterName);
+ }
+ }
+
+ @Override
+ public void setPartitionToError(String clusterName, String instanceName,
String resourceName,
Review Comment:
the code is lot common with 'resetPartition' - can the common code be
separated out and add only the deltas ?
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -1085,6 +1085,139 @@ public ClusterManagementMode
getClusterManagementMode(String clusterName) {
: new ClusterManagementMode(status.getManagementMode(),
status.getManagementModeStatus());
}
+ private enum SetPartitionToErrorFailureReason {
+ INSTANCE_NOT_ALIVE("%s is not alive in cluster %s"),
+ INSTANCE_NON_EXISTENT("%s does not exist in cluster %s"),
+ RESOURCE_NON_EXISTENT("resource %s is not added to cluster %s"),
+ PARTITION_NON_EXISTENT("not all %s exist in cluster %s"),
+ PARTITION_ALREADY_IN_ERROR("%s is NOT found in cluster %s or is already in
ERROR state"),
+ STATE_MODEL_NON_EXISTENT("%s is NOT found in cluster %s");
+
+ private String message;
+
+ SetPartitionToErrorFailureReason(String message) {
+ this.message = message;
+ }
+
+ public String getMessage(String resourceName, List<String> partitionNames,
String instanceName,
+ String errorStateEntity, String clusterName) {
+ return String.format("Can't set to Error State for %s.%s on %s, because
" + message, resourceName,
+ partitionNames, instanceName, errorStateEntity, clusterName);
+ }
+ }
+
+ @Override
+ public void setPartitionToError(String clusterName, String instanceName,
String resourceName,
+ List<String> partitionNames) {
+ logger.info("Set partitions {} for resource {} on instance {} in cluster
{} to ERROR state.",
+ partitionNames == null ? "NULL" :
HelixUtil.serializeByComma(partitionNames), resourceName, instanceName,
+ clusterName);
+ HelixDataAccessor accessor = new ZKHelixDataAccessor(clusterName, new
ZkBaseDataAccessor<ZNRecord>(_zkClient));
+ PropertyKey.Builder keyBuilder = accessor.keyBuilder();
+
+ // check the instance is alive
+ LiveInstance liveInstance =
accessor.getProperty(keyBuilder.liveInstance(instanceName));
+ if (liveInstance == null) {
+ // check if the instance exists in the cluster
+ String instanceConfigPath =
PropertyPathBuilder.instanceConfig(clusterName, instanceName);
+ throw new HelixException(String.format(
+ (_zkClient.exists(instanceConfigPath) ?
SetPartitionToErrorFailureReason.INSTANCE_NOT_ALIVE
+ :
SetPartitionToErrorFailureReason.INSTANCE_NON_EXISTENT).getMessage(resourceName,
partitionNames,
+ instanceName, instanceName, clusterName)));
+ }
+
+ // check resource exists in ideal state
+ IdealState idealState =
accessor.getProperty(keyBuilder.idealStates(resourceName));
+ if (idealState == null) {
+ throw new
HelixException(String.format(SetPartitionToErrorFailureReason.RESOURCE_NON_EXISTENT
+ .getMessage(resourceName, partitionNames, instanceName,
resourceName, clusterName)));
+ }
+
+ // check partition exists in resource
+ Set<String> partitionsToBeSetToError = new HashSet<String>(partitionNames);
+ Set<String> partitions =
Review Comment:
for CUSTOMIZED_MODE - we shouldn't be allowing this, correct?
--
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]