junkaixue commented on code in PR #2100:
URL: https://github.com/apache/helix/pull/2100#discussion_r875143696
##########
helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java:
##########
@@ -86,6 +85,9 @@ public enum ClusterConfigProperty {
// partitons that need recovery or in
// error exceeds this limitation
DISABLED_INSTANCES,
+ DISABLED_INSTANCES_W_INFO,
Review Comment:
NIT: Let's have a full name of it DISABLED_INSTANCES_WITH_INFO.
##########
helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java:
##########
@@ -1118,26 +1138,41 @@ public String getClusterName() {
}
public String getPlainInstanceHelixDisabledType(String instanceName) {
- return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstances().get(instanceName))
+ return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstancesWithInfo().get(instanceName))
.get(ClusterConfigProperty.HELIX_DISABLED_TYPE.toString());
}
public String getInstanceHelixDisabledType(String instanceName) {
- if (!getDisabledInstances().containsKey(instanceName)) {
+ if (!getDisabledInstancesWithInfo().containsKey(instanceName) &&
+ !getDisabledInstances().containsKey(instanceName)) {
return InstanceConstants.INSTANCE_NOT_DISABLED;
}
- return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstances().get(instanceName))
+ return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstancesWithInfo().get(instanceName))
.getOrDefault(ClusterConfigProperty.HELIX_DISABLED_TYPE.toString(),
InstanceConstants.InstanceDisabledType.DEFAULT_INSTANCE_DISABLE_TYPE.toString());
}
+ /**
+ * @return a String representing reason.
+ * null if instance is not disabled in batch mode or do not have disabled
reason
+ */
public String getInstanceHelixDisabledReason(String instanceName) {
- return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstances().get(instanceName))
+ return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstancesWithInfo().get(instanceName))
.get(ClusterConfigProperty.HELIX_DISABLED_REASON.toString());
}
+ /**
+ * @param instanceName
+ * @return a String representation of unix time
+ * null if the instance is not disabled in batch mode.
+ */
public String getInstanceHelixDisabledTimeStamp(String instanceName) {
- return
ConfigStringUtil.parseConcatenatedConfig(getDisabledInstances().get(instanceName))
- .get(ClusterConfigProperty.HELIX_ENABLED_DISABLE_TIMESTAMP.toString());
+ if (getDisabledInstancesWithInfo().containsKey(instanceName)) {
+ return ConfigStringUtil.parseConcatenatedConfig(
+ getDisabledInstancesWithInfo().get(instanceName))
+
.get(ClusterConfigProperty.HELIX_ENABLED_DISABLE_TIMESTAMP.toString());
+ } else {
+ return getDisabledInstances().get(instanceName);
+ }
Review Comment:
NIT: you dont need the else clause.
--
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]
