jiajunwang commented on a change in pull request #1007:
URL: https://github.com/apache/helix/pull/1007#discussion_r427756607
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
##########
@@ -560,6 +560,29 @@ private void processMaintenanceMode(String clusterName,
final boolean enabled,
}
}
+ private enum ResetPartitionFailureReason {
+ INSTANCE_NOTALIVE("%s is not alive in cluster %s"),
+ INSTANCE_NONEXISTENT("%s does not exist in cluster %s"),
+ RESOURCE_NONEXISTENT("resource %s is not added"),
+ PARTITION_NONEXISTENT("not all %s exist"),
+ PARTITION_NOT_ERROR("%s is NOT found"),
+ STATE_MODEL_NONEXISTENT("%s is NOT found"),
+ PENDING_MSG("a pending message exists: %s");
+
+ private String message;
+
+ ResetPartitionFailureReason(String message) {
+ this.message = message;
+ }
+
+ public String getMessage(String instanceName, String resourceName,
Review comment:
The method should either return a formated message or a pure format
string. The current design is mixing and would be hard to use.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
##########
@@ -573,35 +596,31 @@ public void resetPartition(String clusterName, String
instanceName, String resou
// check the instance is alive
LiveInstance liveInstance =
accessor.getProperty(keyBuilder.liveInstance(instanceName));
if (liveInstance == null) {
- throw new HelixException(
- "Can't reset state for " + resourceName + "/" + partitionNames + "
on " + instanceName
- + ", because " + instanceName + " is not alive");
+ // check if the instance has added to the cluster
+ String instanceConfigPath =
PropertyPathBuilder.instanceConfig(clusterName, instanceName);
+ throw new HelixException(String.format(
+ _zkClient.exists(instanceConfigPath) ?
ResetPartitionFailureReason.INSTANCE_NOTALIVE
+ .getMessage(instanceName, resourceName, partitionNames)
+ : ResetPartitionFailureReason.INSTANCE_NONEXISTENT
+ .getMessage(instanceName, resourceName, partitionNames),
instanceName,
+ clusterName));
Review comment:
I would suggest passing the cluster name to getMessage, and let it
return the fully formated string.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
