This is an automated email from the ASF dual-hosted git repository.
jxue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git
The following commit(s) were added to refs/heads/master by this push:
new c46a70f63 Fix incompatible issue for clusterConfig mapfields
disabledInstances (#2100)
c46a70f63 is described below
commit c46a70f6376bb8a217ea3404b8586425314e866b
Author: xyuanlu <[email protected]>
AuthorDate: Wed May 18 13:16:34 2022 -0700
Fix incompatible issue for clusterConfig mapfields disabledInstances (#2100)
Fix incompatible issue for clusterConfig mapfields
---
.../rebalancer/util/DelayedRebalanceUtil.java | 6 +--
.../org/apache/helix/manager/zk/ZKHelixAdmin.java | 15 ++++--
.../java/org/apache/helix/model/ClusterConfig.java | 55 ++++++++++++++++++----
.../apache/helix/util/InstanceValidationUtil.java | 3 +-
.../integration/TestBatchEnableInstances.java | 15 ++++++
.../java/org/apache/helix/mock/MockHelixAdmin.java | 7 ++-
.../helix/rest/server/TestInstancesAccessor.java | 7 +++
7 files changed, 89 insertions(+), 19 deletions(-)
diff --git
a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java
b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java
index 41df15005..4f48b2f93 100644
---
a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java
+++
b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java
@@ -139,10 +139,10 @@ public class DelayedRebalanceUtil {
// check the time instance got disabled.
if (!InstanceValidationUtil.isInstanceEnabled(instanceConfig,
clusterConfig)) {
long disabledTime = instanceConfig.getInstanceEnabledTime();
- Map<String, String> disabledInstances =
clusterConfig.getDisabledInstances();
- if (disabledInstances.containsKey(instance)) {
+ String batchedDisabledTime =
clusterConfig.getInstanceHelixDisabledTimeStamp(instance);
+ if (batchedDisabledTime != null && !batchedDisabledTime.isEmpty()) {
// Update batch disable time
- long batchDisableTime =
Long.parseLong(clusterConfig.getInstanceHelixDisabledTimeStamp(instance));
+ long batchDisableTime = Long.parseLong(batchedDisabledTime);
if (disabledTime == -1 || disabledTime > batchDisableTime) {
disabledTime = batchDisableTime;
}
diff --git
a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
index 673536a89..3e5f2da13 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
@@ -1933,17 +1933,24 @@ public class ZKHelixAdmin implements HelixAdmin {
ClusterConfig clusterConfig = new ClusterConfig(currentData);
Map<String, String> disabledInstances = new
TreeMap<>(clusterConfig.getDisabledInstances());
+ Map<String, String> disabledInstancesWithInfo = new
TreeMap<>(clusterConfig.getDisabledInstancesWithInfo());
if (enabled) {
disabledInstances.keySet().removeAll(instances);
+ disabledInstancesWithInfo.keySet().removeAll(instances);
} else {
for (String disabledInstance : instances) {
// We allow user to override disabledType and reason for an
already disabled instance.
+ // TODO: we are updating both DISABLED_INSTANCES and
DISABLED_INSTANCES_W_INFO for
+ // backward compatible. Deprecate DISABLED_INSTANCES in the future.
// TODO: update the history ZNode
- disabledInstances
- .put(disabledInstance,
assembleInstanceBatchedDisabledInfo(disabledType, reason));
+ String timeStamp = String.valueOf(System.currentTimeMillis());
+ disabledInstances.put(disabledInstance, timeStamp);
+ disabledInstancesWithInfo
+ .put(disabledInstance,
assembleInstanceBatchedDisabledInfo(disabledType, reason, timeStamp));
}
}
clusterConfig.setDisabledInstances(disabledInstances);
+ clusterConfig.setDisabledInstancesWithInfo(disabledInstancesWithInfo);
return clusterConfig.getRecord();
}
@@ -1951,10 +1958,10 @@ public class ZKHelixAdmin implements HelixAdmin {
}
public static String assembleInstanceBatchedDisabledInfo(
- InstanceConstants.InstanceDisabledType disabledType, String reason) {
+ InstanceConstants.InstanceDisabledType disabledType, String reason,
String timeStamp) {
Map<String, String> disableInfo = new TreeMap<>();
disableInfo.put(ClusterConfig.ClusterConfigProperty.HELIX_ENABLED_DISABLE_TIMESTAMP.toString(),
- String.valueOf(System.currentTimeMillis()));
+ timeStamp);
if (disabledType != null) {
disableInfo.put(ClusterConfig.ClusterConfigProperty.HELIX_DISABLED_TYPE.toString(),
disabledType.toString());
diff --git a/helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
b/helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
index 485076ee4..829724142 100644
--- a/helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
+++ b/helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
@@ -34,11 +34,10 @@ import org.apache.helix.HelixProperty;
import org.apache.helix.api.config.HelixConfigProperty;
import org.apache.helix.api.config.StateTransitionThrottleConfig;
import org.apache.helix.api.config.StateTransitionTimeoutConfig;
+import org.apache.helix.api.config.ViewClusterSourceConfig;
import org.apache.helix.constants.InstanceConstants;
import org.apache.helix.util.ConfigStringUtil;
import org.apache.helix.zookeeper.datamodel.ZNRecord;
-import org.apache.helix.api.config.ViewClusterSourceConfig;
-import org.apache.helix.zookeeper.datamodel.ZNRecord;
/**
* Cluster configurations
@@ -86,6 +85,9 @@ public class ClusterConfig extends HelixProperty {
// partitons that need recovery or in
// error exceeds this limitation
DISABLED_INSTANCES,
+ DISABLED_INSTANCES_WITH_INFO,
+ // disabled instances and disabled instances with info are for storing
batch disabled instances.
+ // disabled instances will write into both 2 fields for backward
compatibility.
VIEW_CLUSTER, // Set to "true" to indicate this is a view cluster
VIEW_CLUSTER_SOURCES, // Map field, key is the name of source cluster,
value is
@@ -772,12 +774,32 @@ public class ClusterConfig extends HelixProperty {
_record.setMapField(ClusterConfigProperty.DISABLED_INSTANCES.name(),
disabledInstances);
}
+ /**
+ * Set the disabled instance list with concatenated Info
+ */
+ public void setDisabledInstancesWithInfo(Map<String, String>
disabledInstancesWithInfo) {
+
_record.setMapField(ClusterConfigProperty.DISABLED_INSTANCES_WITH_INFO.name(),
+ disabledInstancesWithInfo);
+ }
+
/**
* Get current disabled instance map of <instance, disabledTimeStamp>
* @return a non-null map of disabled instances in cluster config
*/
public Map<String, String> getDisabledInstances() {
- Map<String, String> disabledInstances =
_record.getMapField(ClusterConfigProperty.DISABLED_INSTANCES.name());
+ Map<String, String> disabledInstances =
+ _record.getMapField(ClusterConfigProperty.DISABLED_INSTANCES.name());
+ return disabledInstances == null ? Collections.emptyMap() :
disabledInstances;
+ }
+
+ /**
+ * Get current disabled instance map of
+ * <instance, disabledReason = "res, disabledType = typ, disabledTimeStamp =
time">
+ * @return a non-null map of disabled instances in cluster config
+ */
+ public Map<String, String> getDisabledInstancesWithInfo() {
+ Map<String, String> disabledInstances =
+
_record.getMapField(ClusterConfigProperty.DISABLED_INSTANCES_WITH_INFO.name());
return disabledInstances == null ? Collections.emptyMap() :
disabledInstances;
}
@@ -1103,7 +1125,6 @@ public class ClusterConfig extends HelixProperty {
}
return idealStateRuleMap;
}
-
@Override
public int hashCode() {
return getId().hashCode();
@@ -1118,26 +1139,40 @@ public class ClusterConfig extends HelixProperty {
}
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());
+ }
+ return getDisabledInstances().get(instanceName);
}
}
diff --git
a/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
b/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
index 6f17d1c49..fa23372f9 100644
--- a/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
+++ b/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
@@ -129,7 +129,8 @@ public class InstanceValidationUtil {
return enabledInInstanceConfig;
}
boolean enabledInClusterConfig =
-
!clusterConfig.getDisabledInstances().containsKey(instanceConfig.getInstanceName());
+
!clusterConfig.getDisabledInstances().containsKey(instanceConfig.getInstanceName())
+ &&
!clusterConfig.getDisabledInstancesWithInfo().containsKey(instanceConfig.getInstanceName());
return enabledInClusterConfig && enabledInInstanceConfig;
}
diff --git
a/helix-core/src/test/java/org/apache/helix/integration/TestBatchEnableInstances.java
b/helix-core/src/test/java/org/apache/helix/integration/TestBatchEnableInstances.java
index 6dfd0037f..6e07a373c 100644
---
a/helix-core/src/test/java/org/apache/helix/integration/TestBatchEnableInstances.java
+++
b/helix-core/src/test/java/org/apache/helix/integration/TestBatchEnableInstances.java
@@ -61,6 +61,12 @@ public class TestBatchEnableInstances extends TaskTestBase {
for (Map<String, String> stateMap :
externalView.getRecord().getMapFields().values()) {
Assert.assertTrue(!stateMap.keySet().contains(_participants[0].getInstanceName()));
}
+ HelixDataAccessor dataAccessor =
+ new ZKHelixDataAccessor(CLUSTER_NAME, new
ZkBaseDataAccessor<>(_gZkClient));
+ ClusterConfig clusterConfig =
dataAccessor.getProperty(dataAccessor.keyBuilder().clusterConfig());
+ Assert.assertEquals(Long.parseLong(
+
clusterConfig.getInstanceHelixDisabledTimeStamp(_participants[0].getInstanceName())),
+
Long.parseLong(clusterConfig.getDisabledInstances().get(_participants[0].getInstanceName())));
_gSetupTool.getClusterManagementTool().enableInstance(CLUSTER_NAME,
_participants[0].getInstanceName(), true);
}
@@ -79,9 +85,18 @@ public class TestBatchEnableInstances extends TaskTestBase {
Assert.assertTrue(!stateMap.keySet().contains(_participants[0].getInstanceName()));
Assert.assertTrue(!stateMap.keySet().contains(_participants[1].getInstanceName()));
}
+ HelixDataAccessor dataAccessor =
+ new ZKHelixDataAccessor(CLUSTER_NAME, new
ZkBaseDataAccessor<>(_gZkClient));
+ ClusterConfig clusterConfig =
dataAccessor.getProperty(dataAccessor.keyBuilder().clusterConfig());
+ Assert.assertEquals(Long.parseLong(
+
clusterConfig.getInstanceHelixDisabledTimeStamp(_participants[1].getInstanceName())),
+
Long.parseLong(clusterConfig.getDisabledInstances().get(_participants[1].getInstanceName())));
_gSetupTool.getClusterManagementTool().enableInstance(CLUSTER_NAME,
Arrays.asList(_participants[0].getInstanceName(),
_participants[1].getInstanceName()),
true);
+ Assert.assertEquals(Long.parseLong(
+
clusterConfig.getInstanceHelixDisabledTimeStamp(_participants[0].getInstanceName())),
+
Long.parseLong(clusterConfig.getDisabledInstances().get(_participants[0].getInstanceName())));
}
@Test
diff --git a/helix-core/src/test/java/org/apache/helix/mock/MockHelixAdmin.java
b/helix-core/src/test/java/org/apache/helix/mock/MockHelixAdmin.java
index 87231d5d7..4063a58ed 100644
--- a/helix-core/src/test/java/org/apache/helix/mock/MockHelixAdmin.java
+++ b/helix-core/src/test/java/org/apache/helix/mock/MockHelixAdmin.java
@@ -319,18 +319,23 @@ public class MockHelixAdmin implements HelixAdmin {
ClusterConfig clusterConfig = new ClusterConfig(record);
Map<String, String> disabledInstances = new TreeMap<>();
+ Map<String, String> disabledInstancesWithInfo = new TreeMap<>();
if (clusterConfig.getDisabledInstances() != null) {
disabledInstances.putAll(clusterConfig.getDisabledInstances());
+
disabledInstancesWithInfo.putAll(clusterConfig.getDisabledInstancesWithInfo());
}
if (enabled) {
disabledInstances.keySet().removeAll(instances);
} else {
for (String disabledInstance : instances) {
+ String timeStamp = String.valueOf(System.currentTimeMillis());
+ disabledInstances.put(disabledInstance, timeStamp);
disabledInstances
- .put(disabledInstance,
assembleInstanceBatchedDisabledInfo(disabledType, reason));
+ .put(disabledInstance,
assembleInstanceBatchedDisabledInfo(disabledType, reason, timeStamp));
}
}
clusterConfig.setDisabledInstances(disabledInstances);
+ clusterConfig.setDisabledInstancesWithInfo(disabledInstancesWithInfo);
}
@Override
diff --git
a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
index 7e9058ff8..87859ffcc 100644
---
a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
+++
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
@@ -154,6 +154,8 @@ public class TestInstancesAccessor extends
AbstractTestClass {
ClusterConfig clusterConfig =
_configAccessor.getClusterConfig(CLUSTER_NAME);
Assert.assertEquals(clusterConfig.getDisabledInstances().keySet(),
new HashSet<>(instancesToDisable));
+ Assert.assertEquals(clusterConfig.getDisabledInstancesWithInfo().keySet(),
+ new HashSet<>(instancesToDisable));
Assert
.assertEquals(clusterConfig.getInstanceHelixDisabledType(CLUSTER_NAME
+ "localhost_12918"),
"USER_OPERATION");
@@ -171,6 +173,11 @@ public class TestInstancesAccessor extends
AbstractTestClass {
clusterConfig = _configAccessor.getClusterConfig(CLUSTER_NAME);
Assert.assertEquals(clusterConfig.getDisabledInstances().keySet(),
new HashSet<>(Arrays.asList(CLUSTER_NAME + "localhost_12919")));
+ Assert.assertEquals(clusterConfig.getDisabledInstancesWithInfo().keySet(),
+ new HashSet<>(Arrays.asList(CLUSTER_NAME + "localhost_12919")));
+ Assert.assertEquals(Long.parseLong(
+ clusterConfig.getInstanceHelixDisabledTimeStamp(CLUSTER_NAME +
"localhost_12919")),
+ Long.parseLong(clusterConfig.getDisabledInstances().get(CLUSTER_NAME +
"localhost_12919")));
Assert
.assertEquals(clusterConfig.getInstanceHelixDisabledType(CLUSTER_NAME
+ "localhost_12918"),
"INSTANCE_NOT_DISABLED");
