Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public enum InstanceConfigProperty {
INSTANCE_INFO_MAP,
INSTANCE_CAPACITY_MAP,
TARGET_TASK_THREAD_POOL_SIZE,
HELIX_INSTANCE_OPERATIONS
HELIX_INSTANCE_OPERATIONS,
INSTANCE_OPERATION_STATE
}

public static class InstanceOperation {
Expand Down Expand Up @@ -249,6 +250,8 @@ private Map<String, String> getProperties() {
*/
public InstanceConfig(String instanceId) {
super(instanceId);
// Initialize the INSTANCE_OPERATION_STATE field
updateInstanceOperationState();
}

/**
Expand All @@ -257,6 +260,10 @@ public InstanceConfig(String instanceId) {
*/
public InstanceConfig(ZNRecord record) {
super(record);
// Ensure the INSTANCE_OPERATION_STATE field is set.
if (_record.getSimpleField(InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()) == null) {
updateInstanceOperationState();
}
}

/**
Expand Down Expand Up @@ -641,6 +648,9 @@ public void setInstanceOperation(InstanceOperation operation) {
serializeInstanceOperations(operations));

setLegacyFieldsForInstanceOperation(operation);

// Update the INSTANCE_OPERATION_STATE field for easy visibility
updateInstanceOperationState();
}

/**
Expand Down Expand Up @@ -683,6 +693,16 @@ private InstanceOperation getActiveInstanceOperation() {
return instanceOperations.get(instanceOperations.size() - 1);
}

/**
* Update the INSTANCE_OPERATION_STATE field based on the current active instance operation.
* This field provides a simple, human-readable view of the current instance state.
*/
private void updateInstanceOperationState() {
InstanceOperation activeOperation = getInstanceOperation();
_record.setSimpleField(InstanceConfigProperty.INSTANCE_OPERATION_STATE.name(),
activeOperation.getOperation().name());
}

/**
* Get the InstanceOperationType of this instance, default is ENABLE if nothing is set. If
* HELIX_ENABLED is set to false, then the instance operation is DISABLE for backwards
Expand Down Expand Up @@ -740,6 +760,23 @@ public InstanceOperation getInstanceOperation() {
return activeInstanceOperation;
}

/**
* Get the current instance operation state. This provides a simple, human-readable view of
* the current instance state (ENABLE, DISABLE, EVACUATE, SWAP_IN, UNKNOWN).
*
* @return the current instance operation state as a string
*/
public String getInstanceOperationState() {
// If the field is not set, compute it from the current instance operation
String state = _record.getSimpleField(InstanceConfigProperty.INSTANCE_OPERATION_STATE.name());
if (state == null || state.isEmpty()) {
state = getInstanceOperation().getOperation().name();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: call updateInstanceOperationState again here

// Update the field so it's persisted
_record.setSimpleField(InstanceConfigProperty.INSTANCE_OPERATION_STATE.name(), state);
}
return state;
}

/**
* Check if this instance is enabled. This is used to determine if the instance can host online
* replicas and take new assignment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,4 +469,143 @@ public void testInstanceOperationMultipleSources() throws InterruptedException {
Assert.assertEquals(instanceConfig.getInstanceOperation().getSource(),
InstanceConstants.InstanceOperationSource.ADMIN);
}

@Test
public void testInstanceOperationStateField() {
// Test 1: New instance should have ENABLE state
InstanceConfig instanceConfig = new InstanceConfig("instance1");
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "ENABLE");
Assert.assertEquals(instanceConfig.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "ENABLE");

// Test 2: Setting DISABLE operation should update state field
instanceConfig.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.DISABLE)
.setReason("test disable")
.build());
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "DISABLE");
Assert.assertEquals(instanceConfig.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "DISABLE");

// Test 3: Create a fresh instance and set EVACUATE operation
InstanceConfig instanceConfig2 = new InstanceConfig("instance2");
instanceConfig2.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.EVACUATE)
.setSource(InstanceConstants.InstanceOperationSource.AUTOMATION)
.build());
Assert.assertEquals(instanceConfig2.getInstanceOperationState(), "EVACUATE");
Assert.assertEquals(instanceConfig2.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "EVACUATE");

// Test 4: Create a fresh instance and set SWAP_IN operation
InstanceConfig instanceConfig3 = new InstanceConfig("instance3");
instanceConfig3.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.SWAP_IN)
.build());
Assert.assertEquals(instanceConfig3.getInstanceOperationState(), "SWAP_IN");
Assert.assertEquals(instanceConfig3.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "SWAP_IN");

// Test 5: Setting ENABLE operation should update state field back to ENABLE
instanceConfig.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.ENABLE)
.build());
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "ENABLE");
Assert.assertEquals(instanceConfig.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "ENABLE");
}

@Test
public void testInstanceOperationStateBackwardCompatibility() {
// Test 1: Loading an old config without INSTANCE_OPERATION_STATE field should auto-populate it
ZNRecord znRecord = new ZNRecord("instance1");
znRecord.setSimpleField(InstanceConfig.InstanceConfigProperty.HELIX_ENABLED.name(), "false");
InstanceConfig instanceConfig = new InstanceConfig(znRecord);

// Should compute state from HELIX_ENABLED field
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "DISABLE");
Assert.assertEquals(instanceConfig.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "DISABLE");

// Test 2: Config with HELIX_ENABLED=true
ZNRecord znRecord2 = new ZNRecord("instance2");
znRecord2.setSimpleField(InstanceConfig.InstanceConfigProperty.HELIX_ENABLED.name(), "true");
InstanceConfig instanceConfig2 = new InstanceConfig(znRecord2);

Assert.assertEquals(instanceConfig2.getInstanceOperationState(), "ENABLE");
Assert.assertEquals(instanceConfig2.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "ENABLE");

// Test 3: Config with HELIX_INSTANCE_OPERATIONS but no INSTANCE_OPERATION_STATE
ZNRecord znRecord3 = new ZNRecord("instance3");
znRecord3.setListField(InstanceConfig.InstanceConfigProperty.HELIX_INSTANCE_OPERATIONS.name(),
List.of("{\"OPERATION\":\"EVACUATE\",\"TIMESTAMP\":\"1733518029415\",\"SOURCE\":\"AUTOMATION\",\"REASON\":\"migration\"}"));
InstanceConfig instanceConfig3 = new InstanceConfig(znRecord3);

Assert.assertEquals(instanceConfig3.getInstanceOperationState(), "EVACUATE");
Assert.assertEquals(instanceConfig3.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "EVACUATE");
}

@Test
public void testInstanceOperationStateWithMultipleSources() {
InstanceConfig instanceConfig = new InstanceConfig("instance1");
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "ENABLE");

// Set DISABLE from USER source
instanceConfig.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.DISABLE)
.setSource(InstanceConstants.InstanceOperationSource.USER)
.setReason("user disabled")
.build());
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "DISABLE");

// Set DISABLE from AUTOMATION source (higher priority, should override)
instanceConfig.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.DISABLE)
.setSource(InstanceConstants.InstanceOperationSource.AUTOMATION)
.setReason("automation disabled")
.build());
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "DISABLE");

// Try to ENABLE from USER source (should not override AUTOMATION)
instanceConfig.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.ENABLE)
.setSource(InstanceConstants.InstanceOperationSource.USER)
.build());
// State should still be DISABLE because AUTOMATION source takes precedence
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "DISABLE");

// ENABLE from AUTOMATION source (should work)
instanceConfig.setInstanceOperation(new InstanceConfig.InstanceOperation.Builder()
.setOperation(InstanceConstants.InstanceOperation.ENABLE)
.setSource(InstanceConstants.InstanceOperationSource.AUTOMATION)
.build());
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "ENABLE");
}

@Test
public void testInstanceOperationStateWithBuilder() {
// Test that Builder properly initializes the state field
InstanceConfig instanceConfig = new InstanceConfig.Builder()
.setHostName("testHost")
.setPort("1234")
.setInstanceOperation(InstanceConstants.InstanceOperation.DISABLE)
.build("instance1");

Assert.assertEquals(instanceConfig.getInstanceOperationState(), "DISABLE");
Assert.assertEquals(instanceConfig.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "DISABLE");

// Test Builder with EVACUATE
InstanceConfig instanceConfig2 = new InstanceConfig.Builder()
.setHostName("testHost2")
.setPort("5678")
.setInstanceOperation(InstanceConstants.InstanceOperation.EVACUATE)
.build("instance2");

Assert.assertEquals(instanceConfig2.getInstanceOperationState(), "EVACUATE");
Assert.assertEquals(instanceConfig2.getRecord().getSimpleField(
InstanceConfig.InstanceConfigProperty.INSTANCE_OPERATION_STATE.name()), "EVACUATE");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -699,13 +699,17 @@ public void updateInstance() throws Exception {
instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, INSTANCE_NAME);
Assert.assertEquals(instanceConfig.getInstanceOperation().getOperation(),
InstanceConstants.InstanceOperation.EVACUATE);
// Verify INSTANCE_OPERATION_STATE field is set correctly
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "EVACUATE");
new JerseyUriRequestBuilder("clusters/{}/instances/{}?command=setInstanceOperation&instanceOperation=INVALIDOP")
.expectedReturnStatusCode(Response.Status.NOT_FOUND.getStatusCode()).format(CLUSTER_NAME, INSTANCE_NAME).post(this, entity);
new JerseyUriRequestBuilder("clusters/{}/instances/{}?command=setInstanceOperation&instanceOperation=")
.format(CLUSTER_NAME, INSTANCE_NAME).post(this, entity);
instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, INSTANCE_NAME);
Assert.assertEquals(instanceConfig.getInstanceOperation().getOperation(),
InstanceConstants.InstanceOperation.ENABLE);
// Verify INSTANCE_OPERATION_STATE field is set correctly
Assert.assertEquals(instanceConfig.getInstanceOperationState(), "ENABLE");

// test canCompleteSwap
Response canCompleteSwapResponse =
Expand Down
Loading