pkuwm commented on a change in pull request #1117:
URL: https://github.com/apache/helix/pull/1117#discussion_r444080381
##########
File path:
helix-core/src/test/java/org/apache/helix/controller/stages/TestRebalancePipeline.java
##########
@@ -203,25 +189,25 @@ public void testMsgTriggeredRebalance() throws Exception {
Assert.assertTrue(TestHelper.verify(() -> {
allMsgs.clear();
for (LiveInstance liveInstance : liveInstances) {
-
allMsgs.addAll(accessor.getChildValues(keyBuilder.messages(liveInstance.getInstanceName()),
- true));
- }
- if (allMsgs.size() != 1 || !allMsgs.get(0).getToState().equals("MASTER")
|| !allMsgs.get(0)
- .getFromState().equals("SLAVE")) {
- return false;
+ allMsgs.addAll(
+
accessor.getChildValues(keyBuilder.messages(liveInstance.getInstanceName()),
true));
}
- return true;
+ // Because current state equals pending message's toState, pending
message is ignored
+ // and ST messages are sent to localhost_0, messages number >= 1
+ return !allMsgs.isEmpty() &&
allMsgs.get(0).getToState().equals("MASTER") && allMsgs.get(0)
Review comment:
This is the change to pass the test, allowing messages size >= 1
##########
File path:
helix-core/src/test/java/org/apache/helix/controller/stages/TestRebalancePipeline.java
##########
@@ -176,22 +163,22 @@ public void testMsgTriggeredRebalance() throws Exception {
// Since controller's rebalancer pipeline will GC pending messages after
timeout, and both hosts
// update current states to SLAVE, controller will send out rebalance
message to
// have one host to become master
- setCurrentState(clusterName, "localhost_0", resourceName, resourceName +
"_0", liveInstances.get(0).getEphemeralOwner(),
- "SLAVE", true);
- setCurrentState(clusterName, "localhost_1", resourceName, resourceName +
"_0", liveInstances.get(1).getEphemeralOwner(),
- "SLAVE", true);
+ setCurrentState(clusterName, "localhost_0", resourceName, resourceName +
"_0",
+ liveInstances.get(0).getEphemeralOwner(), "SLAVE", true);
+ setCurrentState(clusterName, "localhost_1", resourceName, resourceName +
"_0",
+ liveInstances.get(1).getEphemeralOwner(), "SLAVE", true);
// Controller has timeout > 1sec, so within 1s, controller should not have
GCed message
Assert.assertTrue(msgPurgeDelay > 1000);
Assert.assertFalse(TestHelper.verify(() -> {
+ boolean isMessagePurged = false;
for (LiveInstance liveInstance : liveInstances) {
List<String> messages =
accessor.getChildNames(keyBuilder.messages(liveInstance.getInstanceName()));
- if (messages.size() >= 1) {
- return false;
- }
+ // Should verify messages on each live instance are not purged.
+ isMessagePurged = isMessagePurged || messages.isEmpty();
Review comment:
Change 1: verify messages on **each** live instance are not purged.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
