GrantPSpencer commented on code in PR #2848:
URL: https://github.com/apache/helix/pull/2848#discussion_r1690540858
##########
helix-core/src/test/java/org/apache/helix/integration/rebalancer/TestInstanceOperation.java:
##########
@@ -1361,6 +1361,96 @@ public void
testEvacuationWithOfflineInstancesInCluster() throws Exception {
dropTestDBs(ImmutableSet.of("TEST_DB3_DELAYED_CRUSHED",
"TEST_DB4_DELAYED_WAGED"));
}
+ @Test(dependsOnMethods = "testEvacuationWithOfflineInstancesInCluster")
+ public void testEvacuateWithDisabledPartition() throws Exception {
+ System.out.println(
+ "START TestInstanceOperation.testEvacuateWithDisabledPartition() at "
+ new Date(
+ System.currentTimeMillis()));
+ StateTransitionCountStateModelFactory
stateTransitionCountStateModelFactory = new
StateTransitionCountStateModelFactory();
+ removeOfflineOrInactiveInstances();
+ String testCrushedDBName = "testEvacuateWithDisabledPartition_CRUSHED_DB0";
+ String testWagedDBName = "testEvacuateWithDisabledPartition_WAGED_DB1";
+ String toDisableThenEvacuateInstanceName = "disable_then_evacuate_host";
+ addParticipant(toDisableThenEvacuateInstanceName,
stateTransitionCountStateModelFactory);
+ MockParticipantManager toDisableThenEvacuateParticipant =
_participants.get(_participants.size() - 1);
+
+ List<String> testResources = Arrays.asList(testCrushedDBName,
testWagedDBName);
+ createResourceWithDelayedRebalance(CLUSTER_NAME, testCrushedDBName,
"MasterSlave",
+ PARTITIONS, REPLICA, REPLICA-1, 200000,
CrushEdRebalanceStrategy.class.getName());
+ createResourceWithWagedRebalance(CLUSTER_NAME, testWagedDBName,
"MasterSlave", PARTITIONS,
+ REPLICA, REPLICA-1);
+
+ Assert.assertTrue(_clusterVerifier.verifyByPolling());
+ int upwardSTCountBeforeDisableThenEvacuate =
stateTransitionCountStateModelFactory.getUpwardStateTransitionCounter();
+ int downwardSTCountBeforeDisableThenEvacuate =
stateTransitionCountStateModelFactory.getDownwardStateTransitionCounter();
+
+
+ InstanceConfig instanceConfig =
_gSetupTool.getClusterManagementTool().getInstanceConfig(CLUSTER_NAME,
+ toDisableThenEvacuateInstanceName);
+
instanceConfig.setInstanceEnabledForPartition(InstanceConstants.ALL_RESOURCES_DISABLED_PARTITION_KEY,
"", false);
+ _gSetupTool.getClusterManagementTool().setInstanceConfig(CLUSTER_NAME,
toDisableThenEvacuateInstanceName, instanceConfig);
+ Assert.assertTrue(_clusterVerifier.verifyByPolling());
+ // EV should not have disabled instance above the lowest state (OFFLINE)
+ verifier(() -> {
+ for (String resource : testResources) {
+ ExternalView ev =
_gSetupTool.getClusterManagementTool().getResourceExternalView(CLUSTER_NAME,
resource);
+ for (String partition : ev.getPartitionSet()) {
+ if
(ev.getStateMap(partition).containsKey(toDisableThenEvacuateInstanceName) &&
!ev.getStateMap(partition).
+ get(toDisableThenEvacuateInstanceName).equals("OFFLINE")) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }, 5000);
+
+ // Assert node received downward state transitions and no upward
transitions
+
Assert.assertEquals(stateTransitionCountStateModelFactory.getUpwardStateTransitionCounter(),
+ upwardSTCountBeforeDisableThenEvacuate, "Upward state transitions
should not have been received");
+
Assert.assertTrue(stateTransitionCountStateModelFactory.getDownwardStateTransitionCounter()
>
+ downwardSTCountBeforeDisableThenEvacuate, "Should have received
downward state transitions");
+
+ _gSetupTool.getClusterManagementTool().setInstanceOperation(CLUSTER_NAME,
+ toDisableThenEvacuateInstanceName,
InstanceConstants.InstanceOperation.EVACUATE);
+
+ verifier(() -> _admin.isEvacuateFinished(CLUSTER_NAME,
toDisableThenEvacuateInstanceName), 30000);
+
+ // Assert node received no upward state transitions after evacuation was
called on already disabled node
+
Assert.assertEquals(stateTransitionCountStateModelFactory.getUpwardStateTransitionCounter(),
+ upwardSTCountBeforeDisableThenEvacuate, "Upward state transitions
should not have been received");
+
+ // Re-enable all partitions for the instance
+ instanceConfig =
_gSetupTool.getClusterManagementTool().getInstanceConfig(CLUSTER_NAME,
+ toDisableThenEvacuateInstanceName);
+
instanceConfig.setInstanceEnabledForPartition(InstanceConstants.ALL_RESOURCES_DISABLED_PARTITION_KEY,
"", true);
+ _gSetupTool.getClusterManagementTool().setInstanceConfig(CLUSTER_NAME,
toDisableThenEvacuateInstanceName, instanceConfig);
+ Assert.assertTrue(_clusterVerifier.verifyByPolling());
+
+ // Assert node received no upward state transitions after re-enabled
partitions
+
Assert.assertEquals(stateTransitionCountStateModelFactory.getUpwardStateTransitionCounter(),
+ upwardSTCountBeforeDisableThenEvacuate, "Upward state transitions
should not have been received");
+
+ // Disable all partitions for the instance again
+ instanceConfig =
_gSetupTool.getClusterManagementTool().getInstanceConfig(CLUSTER_NAME,
+ toDisableThenEvacuateInstanceName);
+
instanceConfig.setInstanceEnabledForPartition(InstanceConstants.ALL_RESOURCES_DISABLED_PARTITION_KEY,
"", true);
+ _gSetupTool.getClusterManagementTool().setInstanceConfig(CLUSTER_NAME,
toDisableThenEvacuateInstanceName, instanceConfig);
+ Assert.assertTrue(_clusterVerifier.verifyByPolling());
+
+ // Assert node received no upward state transitions after disabling
already evacuated node
+
Assert.assertEquals(stateTransitionCountStateModelFactory.getUpwardStateTransitionCounter(),
Review Comment:
Added assertion that no further downward state transits are experienced when
you disable ALL_RESOURCES on an already evacuated node
--
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: reviews-unsubscr...@helix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
