Skip to content

Enhance getAllInstances API to provide detailed instance operation state categorization #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Enhance getAllInstances API to provide detailed instance operation state categorization #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.helix.HelixAdmin;
import org.apache.helix.HelixDataAccessor;
import org.apache.helix.HelixException;
import org.apache.helix.constants.InstanceConstants;
import org.apache.helix.manager.zk.ZKHelixDataAccessor;
import org.apache.helix.model.ClusterConfig;
import org.apache.helix.model.InstanceConfig;
Expand Down Expand Up @@ -72,6 +73,10 @@ public enum InstancesProperties {
instances,
online,
disabled,
enabled,
evacuated,
swap_in,
unknown,
selection_base,
skip_custom_check_if_instance_not_alive,
zone_order,
Expand Down Expand Up @@ -112,26 +117,57 @@ public Response getAllInstances(@PathParam("clusterId") String clusterId,
ObjectNode root = JsonNodeFactory.instance.objectNode();
root.put(Properties.id.name(), JsonNodeFactory.instance.textNode(clusterId));

// Initialize all arrays
ArrayNode instancesNode =
root.putArray(InstancesAccessor.InstancesProperties.instances.name());
instancesNode.addAll((ArrayNode) OBJECT_MAPPER.valueToTree(instances));

ArrayNode onlineNode = root.putArray(InstancesAccessor.InstancesProperties.online.name());
ArrayNode enabledNode = root.putArray(InstancesAccessor.InstancesProperties.enabled.name());
ArrayNode disabledNode = root.putArray(InstancesAccessor.InstancesProperties.disabled.name());
ArrayNode evacuatedNode = root.putArray(InstancesAccessor.InstancesProperties.evacuated.name());
ArrayNode swapInNode = root.putArray(InstancesAccessor.InstancesProperties.swap_in.name());
ArrayNode unknownNode = root.putArray(InstancesAccessor.InstancesProperties.unknown.name());

List<String> liveInstances = accessor.getChildNames(accessor.keyBuilder().liveInstances());
ClusterConfig clusterConfig = accessor.getProperty(accessor.keyBuilder().clusterConfig());

// Categorize each instance by its operation state
for (String instanceName : instances) {
InstanceConfig instanceConfig =
accessor.getProperty(accessor.keyBuilder().instanceConfig(instanceName));
if (instanceConfig != null) {
if (!InstanceValidationUtil.isInstanceEnabled(instanceConfig, clusterConfig)) {
disabledNode.add(JsonNodeFactory.instance.textNode(instanceName));
}
// Get the instance operation
InstanceConfig.InstanceOperation instanceOperation = instanceConfig.getInstanceOperation();
InstanceConstants.InstanceOperation operation = instanceOperation.getOperation();

// Add to online list if live
if (liveInstances.contains(instanceName)) {
onlineNode.add(JsonNodeFactory.instance.textNode(instanceName));
}

// Categorize by operation state
switch (operation) {
case ENABLE:
enabledNode.add(JsonNodeFactory.instance.textNode(instanceName));
break;
case DISABLE:
disabledNode.add(JsonNodeFactory.instance.textNode(instanceName));
break;
case EVACUATE:
evacuatedNode.add(JsonNodeFactory.instance.textNode(instanceName));
break;
case SWAP_IN:
swapInNode.add(JsonNodeFactory.instance.textNode(instanceName));
break;
case UNKNOWN:
unknownNode.add(JsonNodeFactory.instance.textNode(instanceName));
break;
default:
_logger.warn("Unknown instance operation {} for instance {}. Adding to unknown list.",
operation, instanceName);
unknownNode.add(JsonNodeFactory.instance.textNode(instanceName));
break;
}
}
}
return JSONRepresentation(root);
Expand Down
135 changes: 126 additions & 9 deletions helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,123 @@ public void testGetAllInstances() throws IOException {
System.out.println("End test :" + TestHelper.getTestMethodName());
}

@Test
public void testGetAllInstancesWithOperationStates() throws IOException {
System.out.println("Start test :" + TestHelper.getTestMethodName());

// Get all instances from the cluster
List<String> allInstances = _gSetupTool.getClusterManagementTool().getInstancesInCluster(CLUSTER_NAME);
Assert.assertTrue(allInstances.size() >= 4, "Need at least 4 instances for this test");

// Set different operation states on instances
String enabledInstance = allInstances.get(0);
String disabledInstance = allInstances.get(1);
String evacuatedInstance = allInstances.get(2);
String swapInInstance = allInstances.get(3);

// Set DISABLE operation
InstanceConfig disabledConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, disabledInstance);
disabledConfig.setInstanceOperation(
new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.DISABLE)
.setReason("Test disable")
.build());
_configAccessor.setInstanceConfig(CLUSTER_NAME, disabledInstance, disabledConfig);

// Set EVACUATE operation
InstanceConfig evacuatedConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, evacuatedInstance);
evacuatedConfig.setInstanceOperation(
new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.EVACUATE)
.setReason("Test evacuate")
.build());
_configAccessor.setInstanceConfig(CLUSTER_NAME, evacuatedInstance, evacuatedConfig);

// Set SWAP_IN operation
InstanceConfig swapInConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, swapInInstance);
swapInConfig.setInstanceOperation(
new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.SWAP_IN)
.setReason("Test swap in")
.build());
_configAccessor.setInstanceConfig(CLUSTER_NAME, swapInInstance, swapInConfig);

// Keep one instance with ENABLE (default) - enabledInstance already has ENABLE by default

// Make the API call
String body = new JerseyUriRequestBuilder("clusters/{}/instances").isBodyReturnExpected(true)
.format(CLUSTER_NAME).get(this);

JsonNode node = OBJECT_MAPPER.readTree(body);

// Verify all expected fields are present
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.instances.name()));
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.online.name()));
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.enabled.name()));
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.disabled.name()));
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.evacuated.name()));
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.swap_in.name()));
Assert.assertNotNull(node.get(InstancesAccessor.InstancesProperties.unknown.name()));

// Parse the response arrays
Set<String> enabledInstances = OBJECT_MAPPER.readValue(
node.get(InstancesAccessor.InstancesProperties.enabled.name()).toString(),
OBJECT_MAPPER.getTypeFactory().constructCollectionType(Set.class, String.class));
Set<String> disabledInstances = OBJECT_MAPPER.readValue(
node.get(InstancesAccessor.InstancesProperties.disabled.name()).toString(),
OBJECT_MAPPER.getTypeFactory().constructCollectionType(Set.class, String.class));
Set<String> evacuatedInstances = OBJECT_MAPPER.readValue(
node.get(InstancesAccessor.InstancesProperties.evacuated.name()).toString(),
OBJECT_MAPPER.getTypeFactory().constructCollectionType(Set.class, String.class));
Set<String> swapInInstances = OBJECT_MAPPER.readValue(
node.get(InstancesAccessor.InstancesProperties.swap_in.name()).toString(),
OBJECT_MAPPER.getTypeFactory().constructCollectionType(Set.class, String.class));
Set<String> unknownInstances = OBJECT_MAPPER.readValue(
node.get(InstancesAccessor.InstancesProperties.unknown.name()).toString(),
OBJECT_MAPPER.getTypeFactory().constructCollectionType(Set.class, String.class));

// Verify the categorization is correct
Assert.assertTrue(enabledInstances.contains(enabledInstance),
"Enabled instance should be in enabled list");
Assert.assertTrue(disabledInstances.contains(disabledInstance),
"Disabled instance should be in disabled list");
Assert.assertTrue(evacuatedInstances.contains(evacuatedInstance),
"Evacuated instance should be in evacuated list");
Assert.assertTrue(swapInInstances.contains(swapInInstance),
"Swap-in instance should be in swap_in list");

// Verify instances are not in wrong categories
Assert.assertFalse(disabledInstances.contains(enabledInstance),
"Enabled instance should not be in disabled list");
Assert.assertFalse(evacuatedInstances.contains(disabledInstance),
"Disabled instance should not be in evacuated list");
Assert.assertFalse(enabledInstances.contains(evacuatedInstance),
"Evacuated instance should not be in enabled list");
Assert.assertFalse(enabledInstances.contains(swapInInstance),
"Swap-in instance should not be in enabled list");

// Clean up - reset all instances to ENABLE
disabledConfig.setInstanceOperation(
new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.ENABLE)
.build());
_configAccessor.setInstanceConfig(CLUSTER_NAME, disabledInstance, disabledConfig);

evacuatedConfig.setInstanceOperation(
new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.ENABLE)
.build());
_configAccessor.setInstanceConfig(CLUSTER_NAME, evacuatedInstance, evacuatedConfig);

swapInConfig.setInstanceOperation(
new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.ENABLE)
.build());
_configAccessor.setInstanceConfig(CLUSTER_NAME, swapInInstance, swapInConfig);

System.out.println("End test :" + TestHelper.getTestMethodName());
}

@Test(enabled = false)
public void testUpdateInstances() throws IOException {
// TODO: Reenable the test after storage node fix the problem
Expand Down Expand Up @@ -814,7 +931,7 @@ public void testZoneSelectionBaseWithInstanceThatDontHaveTopologySet() {
public void testInstanceStoppableWithIncludeDetails() throws IOException {
System.out.println("Start test :" + TestHelper.getTestMethodName());

// Test the same scenario as testInstanceStoppableZoneBasedWithToBeStoppedInstances
// Test the same scenario as testInstanceStoppableZoneBasedWithToBeStoppedInstances
// but with includeDetails=true to get enhanced error messages
String content = String.format(
"{\"%s\":\"%s\",\"%s\":[\"%s\"], \"%s\":[\"%s\"]}",
Expand All @@ -830,24 +947,24 @@ public void testInstanceStoppableWithIncludeDetails() throws IOException {

JsonNode nonStoppableInstances = jsonNode.get(
InstancesAccessor.InstancesProperties.instance_not_stoppable_with_reasons.name());
// Instance5 should not be stoppable due to MIN_ACTIVE_REPLICA_CHECK_FAILED

// Instance5 should not be stoppable due to MIN_ACTIVE_REPLICA_CHECK_FAILED
// and should now have detailed information about which partition failed
Set<String> instance5Reasons = getStringSet(nonStoppableInstances, "instance5");
Assert.assertEquals(instance5Reasons.size(), 1);
String reason = instance5Reasons.iterator().next();

// With includeDetails=true, we should get a detailed message
Assert.assertTrue(reason.startsWith("HELIX:MIN_ACTIVE_REPLICA_CHECK_FAILED"),
Assert.assertTrue(reason.startsWith("HELIX:MIN_ACTIVE_REPLICA_CHECK_FAILED"),
"Expected detailed reason to start with HELIX:MIN_ACTIVE_REPLICA_CHECK_FAILED but got: " + reason);

// The detailed message should contain partition information
// Expected format: "HELIX:MIN_ACTIVE_REPLICA_CHECK_FAILED: Resource StoppableTestCluster2_db_0 partition StoppableTestCluster2_db_0_3 has 1/2 active replicas"
Assert.assertTrue(reason.contains("partition"),
Assert.assertTrue(reason.contains("partition"),
"Expected detailed reason to contain partition information but got: " + reason);
Assert.assertTrue(reason.contains("has"),
Assert.assertTrue(reason.contains("has"),
"Expected detailed reason to contain 'has' but got: " + reason);
Assert.assertTrue(reason.contains("active replicas"),
Assert.assertTrue(reason.contains("active replicas"),
"Expected detailed reason to contain 'active replicas' but got: " + reason);

System.out.println("End test :" + TestHelper.getTestMethodName());
Expand Down
Loading