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 2020/01/15 02:38:06 UTC
[GitHub] [helix] i3wangyi opened a new pull request #681: Integration test
for controller connect and disconnect
i3wangyi opened a new pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681
### Issues
- [ ] My PR addresses the following Helix issues and references them in the PR description:
#661
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
(Write a concise description including what, why, how)
Add the integration test to simulate the controller node becomes on/off leader for the test cluster; Also add the case to simulate the leadership switch case.
The integration test will help us to verify interesting properties when the controller is the leader (meaning all callback handlers and zkClient is active, etc) and when the controller is no longer the leader (meaning the callback handlers are no longer active, zkClient should be closed, etc)
- It also serves a quick easy way to figure out the latency on leader -> standby and standby -> leader
### Tests
- [ ] The following tests are written for this issue:
(List the names of added unit/integration tests)
TestControllerLeadershipChange
- testOnSingleController
- testOnLeadershipSwitch
- [ ] The following is the result of the "mvn test" command on the appropriate module:
(Copy & paste the result of "mvn test")
### Commits
- [ ] My commits all reference appropriate Apache Helix GitHub issues in their subject lines, and I have squashed multiple commits if they address the same issue. 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"
### Documentation
- [ ] In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Code Quality
- [ ] My diff has been formatted using helix-style.xml
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r367686371
##########
File path: helix-core/src/main/java/org/apache/helix/HelixTimerTask.java
##########
@@ -32,4 +32,9 @@
* Stop a timer task
*/
public abstract void stop();
+
+ /**
+ * Validate if the timer task is stopped
+ */
+ public abstract boolean isStopped();
Review comment:
One of the options is to cast the HelixTimerTask to a wrapper subclass only visible in test folders. And it assumes the objects in the list will be of type `StatusDumpTask`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r366728978
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -20,29 +20,136 @@
*/
import java.lang.management.ManagementFactory;
+import java.util.List;
import javax.management.MBeanServer;
import javax.management.ObjectName;
import org.apache.helix.AccessOption;
import org.apache.helix.HelixDataAccessor;
import org.apache.helix.HelixManager;
import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.HelixTimerTask;
import org.apache.helix.InstanceType;
import org.apache.helix.PropertyPathBuilder;
import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.manager.zk.CallbackHandler;
+import org.apache.helix.manager.zk.client.HelixZkClient;
import org.apache.helix.model.IdealState;
import org.apache.helix.model.LiveInstance;
import org.apache.helix.monitoring.mbeans.MonitorDomainNames;
import org.apache.helix.tools.ClusterVerifiers.BestPossibleExternalViewVerifier;
import org.apache.helix.tools.ClusterVerifiers.ZkHelixClusterVerifier;
import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+
+/**
+ * Integration test on controller leadership on several phases given the test cluster:
+ * 1. When a standby node becomes the leader node
+ * - single leader node mode
+ * 2. When a leader node becomes standby node
+ * - single leader node mode
+ * 3. When a leader node becomes standby node and the other leader node becomes leader node
+ * - The dual leader nodes mode when the leadership of the test cluster gets changed
+ */
public class TestControllerLeadershipChange extends ZkTestBase {
+ private final String CLASS_NAME = getShortClassName();
+ private final String CLUSTER_NAME = "TestCluster-" + CLASS_NAME;
+
+ @BeforeClass
+ public void beforeClass()
+ throws Exception {
+ super.beforeClass();
+ _gSetupTool.addCluster(CLUSTER_NAME, true);
+ // add some instances
+ _gSetupTool.addInstanceToCluster(CLUSTER_NAME, "TestInstance");
+ _gSetupTool.addResourceToCluster(CLUSTER_NAME, "TestResource", 10, "MasterSlave");
+ }
@Test
- public void testMissingTopStateDurationMonitoring() throws Exception {
+ public void testOnSingleController() {
Review comment:
I think we have the same test case, or a similar test case. For example, TestStandAloneCMMain.java, there is more related test cases. The existing tests are actually more complicated than this one.
I'm fine if you want to re-org the test so they are together. But can we remove the old duplicated tests and ensure we keep the most complicated cases in our test list?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r372646479
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -30,19 +31,157 @@
import org.apache.helix.InstanceType;
import org.apache.helix.PropertyPathBuilder;
import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.manager.zk.CallbackHandler;
+import org.apache.helix.manager.zk.client.HelixZkClient;
import org.apache.helix.model.IdealState;
import org.apache.helix.model.LiveInstance;
import org.apache.helix.monitoring.mbeans.MonitorDomainNames;
import org.apache.helix.tools.ClusterVerifiers.BestPossibleExternalViewVerifier;
import org.apache.helix.tools.ClusterVerifiers.ZkHelixClusterVerifier;
import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+
+/**
+ * Integration test on controller leadership on several phases given the test cluster:
+ * 1. When a standalone controller becomes the leader
+ * 2. When a standalone leader relinquishes the leadership
+ * 3. When the leader node relinquishes the leadership and the other controller takes it over
+ */
public class TestControllerLeadershipChange extends ZkTestBase {
+ private final String CLASS_NAME = getShortClassName();
+ private final String CLUSTER_NAME = "TestCluster-" + CLASS_NAME;
+
+ @BeforeClass
+ public void beforeClass()
+ throws Exception {
+ super.beforeClass();
+ _gSetupTool.addCluster(CLUSTER_NAME, true);
+ _gSetupTool.addInstanceToCluster(CLUSTER_NAME, "TestInstance");
+ _gSetupTool.addResourceToCluster(CLUSTER_NAME, "TestResource", 10, "MasterSlave");
+ }
+
+ @AfterClass
+ public void afterClass() {
+ deleteCluster(CLUSTER_NAME);
+ }
@Test
- public void testMissingTopStateDurationMonitoring() throws Exception {
+ public void testControllerConnectThenDisconnect() {
+ ClusterControllerManager controller =
+ new ClusterControllerManager(ZK_ADDR, CLUSTER_NAME, "TestController");
+ long start = System.currentTimeMillis();
+ controller.syncStart();
+ verifyControllerIsLeader(controller);
+ System.out.println(System.currentTimeMillis() - start + "ms spent on becoming the leader");
+
+ start = System.currentTimeMillis();
+ controller.syncStop();
+ verifyControllerIsNotLeader(controller);
+ verifyZKDisconnected(controller);
+
+ System.out.println(
Review comment:
Please keep the test cases as silent as possible. Only log or throw error asserts on failure case.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r372651000
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -30,19 +31,157 @@
import org.apache.helix.InstanceType;
import org.apache.helix.PropertyPathBuilder;
import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.manager.zk.CallbackHandler;
+import org.apache.helix.manager.zk.client.HelixZkClient;
import org.apache.helix.model.IdealState;
import org.apache.helix.model.LiveInstance;
import org.apache.helix.monitoring.mbeans.MonitorDomainNames;
import org.apache.helix.tools.ClusterVerifiers.BestPossibleExternalViewVerifier;
import org.apache.helix.tools.ClusterVerifiers.ZkHelixClusterVerifier;
import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+
+/**
+ * Integration test on controller leadership on several phases given the test cluster:
+ * 1. When a standalone controller becomes the leader
+ * 2. When a standalone leader relinquishes the leadership
+ * 3. When the leader node relinquishes the leadership and the other controller takes it over
+ */
public class TestControllerLeadershipChange extends ZkTestBase {
+ private final String CLASS_NAME = getShortClassName();
+ private final String CLUSTER_NAME = "TestCluster-" + CLASS_NAME;
+
+ @BeforeClass
+ public void beforeClass()
+ throws Exception {
+ super.beforeClass();
+ _gSetupTool.addCluster(CLUSTER_NAME, true);
+ _gSetupTool.addInstanceToCluster(CLUSTER_NAME, "TestInstance");
+ _gSetupTool.addResourceToCluster(CLUSTER_NAME, "TestResource", 10, "MasterSlave");
+ }
+
+ @AfterClass
+ public void afterClass() {
+ deleteCluster(CLUSTER_NAME);
+ }
@Test
- public void testMissingTopStateDurationMonitoring() throws Exception {
+ public void testControllerConnectThenDisconnect() {
+ ClusterControllerManager controller =
+ new ClusterControllerManager(ZK_ADDR, CLUSTER_NAME, "TestController");
+ long start = System.currentTimeMillis();
+ controller.syncStart();
+ verifyControllerIsLeader(controller);
+ System.out.println(System.currentTimeMillis() - start + "ms spent on becoming the leader");
+
+ start = System.currentTimeMillis();
+ controller.syncStop();
+ verifyControllerIsNotLeader(controller);
+ verifyZKDisconnected(controller);
+
+ System.out.println(
Review comment:
I do need these output of recording the time duration spent on L -> S and S -> L (to prove the improvements). Modify the console print to LOG so it won't be printed out at least during `mvn test`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r366726782
##########
File path: helix-core/src/main/java/org/apache/helix/HelixTimerTask.java
##########
@@ -32,4 +32,9 @@
* Stop a timer task
*/
public abstract void stop();
+
+ /**
+ * Validate if the timer task is stopped
+ */
+ public abstract boolean isStopped();
Review comment:
If this is for test only, please keep it in the implementation classes and keep it protected.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on issue #681: Integration test for
controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on issue #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#issuecomment-579978450
The PR is ready to be merged into Master, approved by @jiajunwang
Commit:
Add integration tests for verifying controller's behavior on/off leadership of a cluster
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r367034758
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -20,29 +20,136 @@
*/
import java.lang.management.ManagementFactory;
+import java.util.List;
import javax.management.MBeanServer;
import javax.management.ObjectName;
import org.apache.helix.AccessOption;
import org.apache.helix.HelixDataAccessor;
import org.apache.helix.HelixManager;
import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.HelixTimerTask;
import org.apache.helix.InstanceType;
import org.apache.helix.PropertyPathBuilder;
import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.manager.zk.CallbackHandler;
+import org.apache.helix.manager.zk.client.HelixZkClient;
import org.apache.helix.model.IdealState;
import org.apache.helix.model.LiveInstance;
import org.apache.helix.monitoring.mbeans.MonitorDomainNames;
import org.apache.helix.tools.ClusterVerifiers.BestPossibleExternalViewVerifier;
import org.apache.helix.tools.ClusterVerifiers.ZkHelixClusterVerifier;
import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+
+/**
+ * Integration test on controller leadership on several phases given the test cluster:
+ * 1. When a standby node becomes the leader node
+ * - single leader node mode
+ * 2. When a leader node becomes standby node
+ * - single leader node mode
+ * 3. When a leader node becomes standby node and the other leader node becomes leader node
+ * - The dual leader nodes mode when the leadership of the test cluster gets changed
+ */
public class TestControllerLeadershipChange extends ZkTestBase {
+ private final String CLASS_NAME = getShortClassName();
+ private final String CLUSTER_NAME = "TestCluster-" + CLASS_NAME;
+
+ @BeforeClass
+ public void beforeClass()
+ throws Exception {
+ super.beforeClass();
+ _gSetupTool.addCluster(CLUSTER_NAME, true);
+ // add some instances
+ _gSetupTool.addInstanceToCluster(CLUSTER_NAME, "TestInstance");
+ _gSetupTool.addResourceToCluster(CLUSTER_NAME, "TestResource", 10, "MasterSlave");
+ }
@Test
- public void testMissingTopStateDurationMonitoring() throws Exception {
+ public void testOnSingleController() {
Review comment:
Thanks for mentioning it! I was unaware of the existing test case at all (it's not trivial to find out). Will merge the similarities part later
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r367685991
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -20,29 +20,136 @@
*/
import java.lang.management.ManagementFactory;
+import java.util.List;
import javax.management.MBeanServer;
import javax.management.ObjectName;
import org.apache.helix.AccessOption;
import org.apache.helix.HelixDataAccessor;
import org.apache.helix.HelixManager;
import org.apache.helix.HelixManagerFactory;
+import org.apache.helix.HelixTimerTask;
import org.apache.helix.InstanceType;
import org.apache.helix.PropertyPathBuilder;
import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.integration.manager.ClusterControllerManager;
import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.manager.zk.CallbackHandler;
+import org.apache.helix.manager.zk.client.HelixZkClient;
import org.apache.helix.model.IdealState;
import org.apache.helix.model.LiveInstance;
import org.apache.helix.monitoring.mbeans.MonitorDomainNames;
import org.apache.helix.tools.ClusterVerifiers.BestPossibleExternalViewVerifier;
import org.apache.helix.tools.ClusterVerifiers.ZkHelixClusterVerifier;
import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+
+/**
+ * Integration test on controller leadership on several phases given the test cluster:
+ * 1. When a standby node becomes the leader node
+ * - single leader node mode
+ * 2. When a leader node becomes standby node
+ * - single leader node mode
+ * 3. When a leader node becomes standby node and the other leader node becomes leader node
+ * - The dual leader nodes mode when the leadership of the test cluster gets changed
+ */
public class TestControllerLeadershipChange extends ZkTestBase {
+ private final String CLASS_NAME = getShortClassName();
+ private final String CLUSTER_NAME = "TestCluster-" + CLASS_NAME;
+
+ @BeforeClass
+ public void beforeClass()
+ throws Exception {
+ super.beforeClass();
+ _gSetupTool.addCluster(CLUSTER_NAME, true);
+ // add some instances
+ _gSetupTool.addInstanceToCluster(CLUSTER_NAME, "TestInstance");
+ _gSetupTool.addResourceToCluster(CLUSTER_NAME, "TestResource", 10, "MasterSlave");
+ }
@Test
- public void testMissingTopStateDurationMonitoring() throws Exception {
+ public void testOnSingleController() {
Review comment:
Synced offline. We both agree that the existing test is outdated and already broken. The new test case will cover the one since it's deleted in the PR
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r372650313
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -117,16 +258,17 @@ public void testMissingTopStateDurationMonitoring() throws Exception {
// Resource lost top state, and manager1 lost leadership for 2000ms, because manager1 will
// clean monitoring cache after re-gaining leadership, so max value of hand off duration should
// not have such a large value
- Assert.assertTrue((long) beanServer.getAttribute(resourceMBeanObjectName,
- "PartitionTopStateHandoffDurationGauge.Max") < 500);
+ Assert.assertTrue((long) beanServer
+ .getAttribute(resourceMBeanObjectName, "PartitionTopStateHandoffDurationGauge.Max") < 500);
participant.syncStop();
manager1.disconnect();
manager2.disconnect();
deleteCluster(clusterName);
}
- private void setLeader(HelixManager manager) throws Exception {
+ private void setLeader(HelixManager manager)
Review comment:
good catch. updated
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] pkuwm commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r366697066
##########
File path: helix-core/src/main/java/org/apache/helix/HelixTimerTask.java
##########
@@ -32,4 +32,9 @@
* Stop a timer task
*/
public abstract void stop();
+
+ /**
+ * Validate if the timer task is stopped
+ */
+ public abstract boolean isStopped();
Review comment:
Would this break a class which extends HelixTimerTask in an external package?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang merged pull request #681: Integration test for
controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
jiajunwang merged pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r372647423
##########
File path: helix-core/src/test/java/org/apache/helix/integration/controller/TestControllerLeadershipChange.java
##########
@@ -117,16 +258,17 @@ public void testMissingTopStateDurationMonitoring() throws Exception {
// Resource lost top state, and manager1 lost leadership for 2000ms, because manager1 will
// clean monitoring cache after re-gaining leadership, so max value of hand off duration should
// not have such a large value
- Assert.assertTrue((long) beanServer.getAttribute(resourceMBeanObjectName,
- "PartitionTopStateHandoffDurationGauge.Max") < 500);
+ Assert.assertTrue((long) beanServer
+ .getAttribute(resourceMBeanObjectName, "PartitionTopStateHandoffDurationGauge.Max") < 500);
participant.syncStop();
manager1.disconnect();
manager2.disconnect();
deleteCluster(clusterName);
}
- private void setLeader(HelixManager manager) throws Exception {
+ private void setLeader(HelixManager manager)
Review comment:
nit, I believe the latest style file has changed exception throws to non-wrapping. Please update your local style configuration.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] i3wangyi commented on a change in pull request #681:
Integration test for controller connect and disconnect
Posted by GitBox <gi...@apache.org>.
i3wangyi commented on a change in pull request #681: Integration test for controller connect and disconnect
URL: https://github.com/apache/helix/pull/681#discussion_r367036148
##########
File path: helix-core/src/main/java/org/apache/helix/HelixTimerTask.java
##########
@@ -32,4 +32,9 @@
* Stop a timer task
*/
public abstract void stop();
+
+ /**
+ * Validate if the timer task is stopped
+ */
+ public abstract boolean isStopped();
Review comment:
The only exposed interface from ZkHelixManager is `List<HelixTimerTask> _controllerTimerTasks = xxx` and the actual implementation class is `static class StatusDumpTask` which is not exposed neither. How could I create a protected method and make it only visible for the test?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org