You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2021/07/13 07:56:37 UTC
[GitHub] [helix] huizhilu opened a new pull request #1816: Add integration tests for cluster freeze mode
huizhilu opened a new pull request #1816:
URL: https://github.com/apache/helix/pull/1816
### Issues
- [ ] My PR addresses the following Helix issues and references them in the PR description:
Resolves #1815
This PR replies on #1812 to freeze participants.
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
This PR adds integration tests for cluster freeze mode:
a. freeze cluster
b. simulate a live instance is rebooted and back to online - it should be frozen
c. unfreeze cluster
Adding more...
### Tests
- [ ] The following tests are written for this issue:
`TestClusterFreezeMode`
- The following is the result of the "mvn test" command on the appropriate module:
(If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)
### Changes that Break Backward Compatibility (Optional)
- My PR contains changes that break backward compatibility or previous assumptions for certain methods or API. They include:
(Consider including all behavior changes for public methods or API. Also include these changes in merge description so that other developers are aware of these changes. This allows them to make relevant code changes in feature branches accounting for the new method/API behavior.)
### Documentation (Optional)
- In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Commits
- My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1816:
URL: https://github.com/apache/helix/pull/1816#discussion_r668995457
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
Review comment:
Before this step, better to test the intermediate state. Block one participant and let it wait for some time, and then check if the overall status is in progress to frozen.
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ ClusterStatus expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.CLUSTER_PAUSE);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
+ verifyClusterStatus(expectedClusterStatus);
+
+ // Add a new live instance. Simulate an instance is rebooted and back to online
+ String newInstanceName = "localhost_" + (12918 + _numNodes + 1);
+ _gSetupTool.addInstancesToCluster(_clusterName, new String[]{newInstanceName});
+ MockParticipantManager newParticipant =
+ new MockParticipantManager(ZK_ADDR, _clusterName, newInstanceName);
+ newParticipant.syncStart();
+
+ // The new participant/live instance should be frozen by controller
+ verifyLiveInstanceStatus(new MockParticipantManager[]{newParticipant},
+ LiveInstance.LiveInstanceStatus.PAUSED);
+
+ newParticipant.syncStop();
+
+ // Unfreeze cluster
+ request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.NORMAL)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.NORMAL);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
Review comment:
Question, what if a cluster has never been frozen, will the Mode status also be COMPLETED?
##########
File path: helix-core/src/main/java/org/apache/helix/controller/stages/ManagementMessageGenerationPhase.java
##########
@@ -96,7 +96,9 @@ public void process(ClusterEvent event) throws Exception {
LiveInstance liveInstance = liveInstanceMap.get(instanceName);
Collection<Message> pendingMessages = allInstanceMessages.get(instanceName);
String sessionId = liveInstance.getEphemeralOwner();
- LiveInstanceStatus currentStatus = liveInstance.getStatus();
+ LiveInstanceStatus liveInstanceStatus = liveInstance.getStatus();
Review comment:
IMHO, getStatus() returns NORMAL directly would be easier.
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ ClusterStatus expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.CLUSTER_PAUSE);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
+ verifyClusterStatus(expectedClusterStatus);
+
+ // Add a new live instance. Simulate an instance is rebooted and back to online
+ String newInstanceName = "localhost_" + (12918 + _numNodes + 1);
+ _gSetupTool.addInstancesToCluster(_clusterName, new String[]{newInstanceName});
+ MockParticipantManager newParticipant =
+ new MockParticipantManager(ZK_ADDR, _clusterName, newInstanceName);
+ newParticipant.syncStart();
+
+ // The new participant/live instance should be frozen by controller
+ verifyLiveInstanceStatus(new MockParticipantManager[]{newParticipant},
+ LiveInstance.LiveInstanceStatus.PAUSED);
+
+ newParticipant.syncStop();
+
+ // Unfreeze cluster
+ request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.NORMAL)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
Review comment:
If the request is done really fast (before this line), will this check fail?
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ ClusterStatus expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.CLUSTER_PAUSE);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
+ verifyClusterStatus(expectedClusterStatus);
+
+ // Add a new live instance. Simulate an instance is rebooted and back to online
+ String newInstanceName = "localhost_" + (12918 + _numNodes + 1);
+ _gSetupTool.addInstancesToCluster(_clusterName, new String[]{newInstanceName});
+ MockParticipantManager newParticipant =
+ new MockParticipantManager(ZK_ADDR, _clusterName, newInstanceName);
+ newParticipant.syncStart();
+
+ // The new participant/live instance should be frozen by controller
+ verifyLiveInstanceStatus(new MockParticipantManager[]{newParticipant},
+ LiveInstance.LiveInstanceStatus.PAUSED);
+
+ newParticipant.syncStop();
+
+ // Unfreeze cluster
Review comment:
Before test unfreeze,
1. Shall we test freezing a cluster with ongoing STs?
2. Test creating new resources during the freeze mode, please.
3. Test resetting an existing partition on a paused participant, please.
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ ClusterStatus expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.CLUSTER_PAUSE);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
+ verifyClusterStatus(expectedClusterStatus);
+
+ // Add a new live instance. Simulate an instance is rebooted and back to online
Review comment:
Please also test restart/reenable an existing instance and see if it behave correctly.
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
Review comment:
I think you will need to validate if the history is recorded accurately or not.
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] huizhilu commented on pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
huizhilu commented on pull request #1816:
URL: https://github.com/apache/helix/pull/1816#issuecomment-880241506
This PR is ready to be merged, approved by @jiajunwang
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] huizhilu commented on a change in pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
huizhilu commented on a change in pull request #1816:
URL: https://github.com/apache/helix/pull/1816#discussion_r669011681
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ ClusterStatus expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.CLUSTER_PAUSE);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
+ verifyClusterStatus(expectedClusterStatus);
+
+ // Add a new live instance. Simulate an instance is rebooted and back to online
+ String newInstanceName = "localhost_" + (12918 + _numNodes + 1);
+ _gSetupTool.addInstancesToCluster(_clusterName, new String[]{newInstanceName});
+ MockParticipantManager newParticipant =
+ new MockParticipantManager(ZK_ADDR, _clusterName, newInstanceName);
+ newParticipant.syncStart();
+
+ // The new participant/live instance should be frozen by controller
+ verifyLiveInstanceStatus(new MockParticipantManager[]{newParticipant},
+ LiveInstance.LiveInstanceStatus.PAUSED);
+
+ newParticipant.syncStop();
+
+ // Unfreeze cluster
+ request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.NORMAL)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.NORMAL);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
Review comment:
Yes. It's completed in NORMAL mode.
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1816:
URL: https://github.com/apache/helix/pull/1816#discussion_r669972905
##########
File path: helix-core/src/main/java/org/apache/helix/controller/stages/ManagementMessageGenerationPhase.java
##########
@@ -96,7 +96,9 @@ public void process(ClusterEvent event) throws Exception {
LiveInstance liveInstance = liveInstanceMap.get(instanceName);
Collection<Message> pendingMessages = allInstanceMessages.get(instanceName);
String sessionId = liveInstance.getEphemeralOwner();
- LiveInstanceStatus currentStatus = liveInstance.getStatus();
+ LiveInstanceStatus liveInstanceStatus = liveInstance.getStatus();
Review comment:
There was no convention about how a HelixProperty class should handle such a case (missing field). So we have both types of examples through our codebase. For the future, I think we should make all these different cases aligned.
1. From my perspective, each HelixProperty class should carry more business logic. And let the ZNode class to handle the raw content. So missing field can be returned from ZNode or it's methods. But should not be returned by the HelixProperty unless it is representing a specific business logic.
2. Based on point one, do you think null LiveInstanceStatus is representing any business logic? I think return NORMAL when this field is missing would be quite straightforward and is not confusing at all. Moreover, it is fully backward compatible. The older participant does not have any special status, so they will be NORMAL forever.
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] huizhilu commented on a change in pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
huizhilu commented on a change in pull request #1816:
URL: https://github.com/apache/helix/pull/1816#discussion_r669986519
##########
File path: helix-core/src/main/java/org/apache/helix/controller/stages/ManagementMessageGenerationPhase.java
##########
@@ -96,7 +96,9 @@ public void process(ClusterEvent event) throws Exception {
LiveInstance liveInstance = liveInstanceMap.get(instanceName);
Collection<Message> pendingMessages = allInstanceMessages.get(instanceName);
String sessionId = liveInstance.getEphemeralOwner();
- LiveInstanceStatus currentStatus = liveInstance.getStatus();
+ LiveInstanceStatus liveInstanceStatus = liveInstance.getStatus();
Review comment:
Synced offline. Returning null or normal isn't a big issue. As @jiajunwang said, to simplify the business logic, I'll make the change to return NORMAL in next PR, combined with the refactoring work, making it easy to review.
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] huizhilu commented on a change in pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
huizhilu commented on a change in pull request #1816:
URL: https://github.com/apache/helix/pull/1816#discussion_r669327409
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
Review comment:
Done. There is pending state transition message blocking there, so it is IN_PROGRESS to FREEZE.
##########
File path: helix-core/src/main/java/org/apache/helix/controller/stages/ManagementMessageGenerationPhase.java
##########
@@ -96,7 +96,9 @@ public void process(ClusterEvent event) throws Exception {
LiveInstance liveInstance = liveInstanceMap.get(instanceName);
Collection<Message> pendingMessages = allInstanceMessages.get(instanceName);
String sessionId = liveInstance.getEphemeralOwner();
- LiveInstanceStatus currentStatus = liveInstance.getStatus();
+ LiveInstanceStatus liveInstanceStatus = liveInstance.getStatus();
Review comment:
It could easier, but might be confusing. If by default returning NORMAL, we cannot differentiate if the status is set to NORMAL or not set. To make the current logic consistent, I'll just keep getStatus as it is: returning NULL(not set).
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] huizhilu commented on a change in pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
huizhilu commented on a change in pull request #1816:
URL: https://github.com/apache/helix/pull/1816#discussion_r669014271
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestClusterFreezeMode.java
##########
@@ -0,0 +1,171 @@
+package org.apache.helix.integration.controller;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Arrays;
+
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.InstanceType;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.TestHelper;
+import org.apache.helix.api.status.ClusterManagementMode;
+import org.apache.helix.api.status.ClusterManagementModeRequest;
+import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.ClusterManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.model.ClusterStatus;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.PauseSignal;
+import org.apache.helix.tools.ClusterStateVerifier;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class TestClusterFreezeMode extends ZkTestBase {
+ private HelixManager _manager;
+ private HelixDataAccessor _accessor;
+ private String _clusterName;
+ private int _numNodes;
+ private MockParticipantManager[] _participants;
+ private ClusterControllerManager _controller;
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ _numNodes = 5;
+ _clusterName = "CLUSTER_" + TestHelper.getTestClassName();
+ _participants = new MockParticipantManager[_numNodes];
+ TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port
+ "localhost", // participant name prefix
+ "TestDB", // resource name prefix
+ 1, // resources
+ 10, // partitions per resource
+ _numNodes, // number of nodes
+ 3, // replicas
+ "MasterSlave", true);
+
+ _manager = HelixManagerFactory
+ .getZKHelixManager(_clusterName, "Admin", InstanceType.ADMINISTRATOR, ZK_ADDR);
+ _manager.connect();
+ _accessor = _manager.getHelixDataAccessor();
+
+ // start controller
+ _controller = new ClusterControllerManager(ZK_ADDR, _clusterName, "controller_0");
+ _controller.syncStart();
+
+ // start participants
+ for (int i = 0; i < 5; i++) {
+ String instanceName = "localhost_" + (12918 + i);
+ _participants[i] = new MockParticipantManager(ZK_ADDR, _clusterName, instanceName);
+ _participants[i].syncStart();
+ }
+
+ boolean result = ClusterStateVerifier.verifyByZkCallback(
+ new ClusterStateVerifier.BestPossAndExtViewZkVerifier(ZK_ADDR, _clusterName));
+ Assert.assertTrue(result);
+ }
+
+ @AfterClass
+ public void afterClass() {
+ _manager.disconnect();
+ _controller.syncStop();
+ Arrays.stream(_participants).forEach(ClusterManager::syncStop);
+ deleteCluster(_clusterName);
+ }
+
+ @Test
+ public void testEnableFreezeMode() throws Exception {
+ // Not in freeze mode
+ PauseSignal pauseSignal = _accessor.getProperty(_accessor.keyBuilder().pause());
+ Assert.assertNull(pauseSignal);
+
+ // Freeze cluster
+ ClusterManagementModeRequest request = ClusterManagementModeRequest.newBuilder()
+ .withClusterName(_clusterName)
+ .withMode(ClusterManagementMode.Type.CLUSTER_PAUSE)
+ .withReason("test")
+ .build();
+ _gSetupTool.getClusterManagementTool().setClusterManagementMode(request);
+
+ // Verify live instance status and cluster status
+ verifyLiveInstanceStatus(_participants, LiveInstance.LiveInstanceStatus.PAUSED);
+
+ ClusterStatus expectedClusterStatus = new ClusterStatus();
+ expectedClusterStatus.setManagementMode(ClusterManagementMode.Type.CLUSTER_PAUSE);
+ expectedClusterStatus.setManagementModeStatus(ClusterManagementMode.Status.COMPLETED);
+ verifyClusterStatus(expectedClusterStatus);
+
+ // Add a new live instance. Simulate an instance is rebooted and back to online
Review comment:
Yes. This PR is a draft. Still adding more cases like you said, pending ST, restart an existing instance, checking history, etc..
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] huizhilu merged pull request #1816: Add integration tests for cluster freeze mode
Posted by GitBox <gi...@apache.org>.
huizhilu merged pull request #1816:
URL: https://github.com/apache/helix/pull/1816
--
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-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org