junkaixue commented on code in PR #2618:
URL: https://github.com/apache/helix/pull/2618#discussion_r1329388218
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -406,7 +407,41 @@ public ZNRecord update(ZNRecord currentData) {
}
}
+
@Override
+ public boolean isEvacuateFinished(String clusterName, String instanceName) {
+ 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) {
+ logger.warn("instance {} in cluster {} is not alive.", instanceName,
clusterName);
+ return false;
Review Comment:
Let's make it true as it designed for whether we can continue following
operation when the evacuated is done.
##########
helix-core/src/main/java/org/apache/helix/HelixAdmin.java:
##########
@@ -738,4 +738,15 @@ Map<String, Boolean>
validateResourcesForWagedRebalance(String clusterName,
*/
Map<String, Boolean> validateInstancesForWagedRebalance(String clusterName,
List<String> instancesNames);
+
+ /**
+ * Return if instance operation 'Evacuate' is finished.
+ * Only return true if there is no current state on the instance and that
instance is still alive.
+ * @param clusterName
+ * @param instancesNames
+ * @return
+ */
+ boolean isEvacuateFinished(String clusterName, String instancesNames);
+
+ boolean isPrepopulateReady(String clusterName, String instancesNames);
Review Comment:
java doc
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -406,7 +407,41 @@ public ZNRecord update(ZNRecord currentData) {
}
}
+
@Override
+ public boolean isEvacuateFinished(String clusterName, String instanceName) {
+ 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) {
+ logger.warn("instance {} in cluster {} is not alive.", instanceName,
clusterName);
+ return false;
+ }
+
+ BaseDataAccessor<ZNRecord> baseAccessor = new
ZkBaseDataAccessor<ZNRecord>(_zkClient);
+ String sessionId = liveInstance.getEphemeralOwner();
+
+ String path = PropertyPathBuilder.instanceCurrentState(clusterName,
instanceName, sessionId);
+ List<String> currentStates = baseAccessor.getChildNames(path, 0);
+ if (currentStates == null) {
+ logger.warn("instance {} in cluster {} does not have live session.",
instanceName,
+ clusterName);
+ return false;
+ }
+ return currentStates.isEmpty();
Review Comment:
edge case... check whether has pending messages.
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -406,7 +407,41 @@ public ZNRecord update(ZNRecord currentData) {
}
}
+
@Override
+ public boolean isEvacuateFinished(String clusterName, String instanceName) {
+ 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) {
+ logger.warn("instance {} in cluster {} is not alive.", instanceName,
clusterName);
+ return false;
+ }
+
+ BaseDataAccessor<ZNRecord> baseAccessor = new
ZkBaseDataAccessor<ZNRecord>(_zkClient);
+ String sessionId = liveInstance.getEphemeralOwner();
+
+ String path = PropertyPathBuilder.instanceCurrentState(clusterName,
instanceName, sessionId);
+ List<String> currentStates = baseAccessor.getChildNames(path, 0);
+ if (currentStates == null) {
+ logger.warn("instance {} in cluster {} does not have live session.",
instanceName,
+ clusterName);
+ return false;
+ }
+ return currentStates.isEmpty();
+ }
+
+ @Override
+ public boolean isPrepopulateReady(String clusterName, String instanceName) {
Review Comment:
rename it more clear.
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -406,7 +407,41 @@ public ZNRecord update(ZNRecord currentData) {
}
}
+
@Override
+ public boolean isEvacuateFinished(String clusterName, String instanceName) {
+ 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) {
+ logger.warn("instance {} in cluster {} is not alive.", instanceName,
clusterName);
+ return false;
+ }
+
+ BaseDataAccessor<ZNRecord> baseAccessor = new
ZkBaseDataAccessor<ZNRecord>(_zkClient);
+ String sessionId = liveInstance.getEphemeralOwner();
+
+ String path = PropertyPathBuilder.instanceCurrentState(clusterName,
instanceName, sessionId);
+ List<String> currentStates = baseAccessor.getChildNames(path, 0);
+ if (currentStates == null) {
+ logger.warn("instance {} in cluster {} does not have live session.",
instanceName,
+ clusterName);
+ return false;
+ }
+ return currentStates.isEmpty();
+ }
+
+ @Override
+ public boolean isPrepopulateReady(String clusterName, String instanceName) {
+ return this.getInstanceConfig(clusterName, instanceName)
+ .getInstanceOperation()
+ .equals(InstanceConstants.InstanceOperation.EVACUATE.name());
Review Comment:
Be consistent with the operation tag. Add SWAP_OUT here.
--
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]