jackjlli commented on a change in pull request #4430: Cleanup un-necessary
setups in HelixSetupUtils
URL: https://github.com/apache/incubator-pinot/pull/4430#discussion_r303183403
##########
File path:
pinot-controller/src/test/java/org/apache/pinot/controller/helix/PinotControllerModeTest.java
##########
@@ -32,183 +32,185 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_REPLICA_COUNT;
+
public class PinotControllerModeTest extends ControllerTest {
private static long TIMEOUT_IN_MS = 10_000L;
- private ControllerConf config;
- private int controllerPortOffset;
@BeforeClass
public void setUp() {
startZk();
- config = getDefaultControllerConfiguration();
- controllerPortOffset = 0;
}
@Test
- public void testHelixOnlyController()
- throws Exception {
- config.setControllerMode(ControllerConf.ControllerMode.HELIX_ONLY);
-
config.setControllerPort(Integer.toString(Integer.parseInt(config.getControllerPort())
+ controllerPortOffset++));
-
- startController(config);
- TestUtils.waitForCondition(aVoid -> _helixManager.isConnected(),
TIMEOUT_IN_MS,
- "Failed to start " + config.getControllerMode() + " controller in " +
TIMEOUT_IN_MS + "ms.");
-
- Assert.assertEquals(_controllerStarter.getControllerMode(),
ControllerConf.ControllerMode.HELIX_ONLY);
-
- stopController();
- _controllerStarter = null;
+ public void testHelixOnlyController() {
+ ControllerConf helixOnlyControllerConfig =
getDefaultControllerConfiguration();
+
helixOnlyControllerConfig.setControllerMode(ControllerConf.ControllerMode.HELIX_ONLY);
+ ControllerStarter helixOnlyController =
getControllerStarter(helixOnlyControllerConfig);
+ helixOnlyController.start();
+ TestUtils.waitForCondition(aVoid ->
helixOnlyController.getHelixControllerManager().isConnected(), TIMEOUT_IN_MS,
+ "Failed to start the Helix-only controller");
+
+ helixOnlyController.stop();
}
@Test
- public void testDualModeController()
- throws Exception {
- config.setControllerMode(ControllerConf.ControllerMode.DUAL);
-
config.setControllerPort(Integer.toString(Integer.parseInt(config.getControllerPort())
+ controllerPortOffset++));
-
+ public void testDualModeController() {
// Helix cluster will be set up when starting the first controller.
- startController(config);
- TestUtils.waitForCondition(aVoid -> _helixManager.isConnected(),
TIMEOUT_IN_MS,
- "Failed to start " + config.getControllerMode() + " controller in " +
TIMEOUT_IN_MS + "ms.");
- Assert.assertEquals(_controllerStarter.getControllerMode(),
ControllerConf.ControllerMode.DUAL);
-
- // Enable the lead controller resource.
- _helixAdmin.enableResource(getHelixClusterName(),
CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME, true);
+ ControllerConf firstDualModeControllerConfig =
getDefaultControllerConfiguration();
+
firstDualModeControllerConfig.setControllerMode(ControllerConf.ControllerMode.DUAL);
+ ControllerStarter firstDualModeController =
getControllerStarter(firstDualModeControllerConfig);
+ firstDualModeController.start();
+ TestUtils
+ .waitForCondition(aVoid ->
firstDualModeController.getHelixControllerManager().isConnected(),
TIMEOUT_IN_MS,
+ "Failed to start the first dual-mode controller");
// Starting a second dual-mode controller. Helix cluster has already been
set up.
- ControllerConf controllerConfig = getDefaultControllerConfiguration();
- controllerConfig.setHelixClusterName(getHelixClusterName());
- controllerConfig.setControllerMode(ControllerConf.ControllerMode.DUAL);
- controllerConfig.setControllerPort(
- Integer.toString(Integer.parseInt(this.config.getControllerPort()) +
controllerPortOffset++));
-
- ControllerStarter secondDualModeController = new
TestOnlyControllerStarter(controllerConfig);
+ ControllerConf secondDualModeControllerConfig =
getDefaultControllerConfiguration();
+
secondDualModeControllerConfig.setControllerMode(ControllerConf.ControllerMode.DUAL);
+
secondDualModeControllerConfig.setControllerPort(Integer.toString(DEFAULT_CONTROLLER_PORT
+ 1));
+ ControllerStarter secondDualModeController =
getControllerStarter(secondDualModeControllerConfig);
secondDualModeController.start();
TestUtils
.waitForCondition(aVoid ->
secondDualModeController.getHelixResourceManager().getHelixZkManager().isConnected(),
- TIMEOUT_IN_MS, "Failed to start " + config.getControllerMode() + "
controller in " + TIMEOUT_IN_MS + "ms.");
- Assert.assertEquals(secondDualModeController.getControllerMode(),
ControllerConf.ControllerMode.DUAL);
+ TIMEOUT_IN_MS, "Failed to start the second dual-mode controller");
secondDualModeController.stop();
- stopController();
- _controllerStarter = null;
+ firstDualModeController.stop();
}
// TODO: enable it after removing ControllerLeadershipManager which requires
both CONTROLLER and PARTICIPANT
// HelixManager
@Test(enabled = false)
- public void testPinotOnlyController()
- throws Exception {
- config.setControllerMode(ControllerConf.ControllerMode.PINOT_ONLY);
-
config.setControllerPort(Integer.toString(Integer.parseInt(config.getControllerPort())
+ controllerPortOffset++));
+ public void testPinotOnlyController() {
+ ControllerConf firstPinotOnlyControllerConfig =
getDefaultControllerConfiguration();
+
firstPinotOnlyControllerConfig.setControllerMode(ControllerConf.ControllerMode.PINOT_ONLY);
+ ControllerStarter firstPinotOnlyController =
getControllerStarter(firstPinotOnlyControllerConfig);
// Starting pinot only controller before starting helix controller should
fail.
try {
- startController(config);
- Assert.fail("Starting pinot only controller should fail!");
- } catch (RuntimeException e) {
- _controllerStarter = null;
+ firstPinotOnlyController.start();
+ Assert.fail("Starting Pinot-only controller without Helix controller
should fail");
+ } catch (Exception e) {
+ // Expected
}
// Starting a helix controller.
- ControllerConf config2 = getDefaultControllerConfiguration();
- config2.setHelixClusterName(getHelixClusterName());
- config2.setControllerMode(ControllerConf.ControllerMode.HELIX_ONLY);
-
config2.setControllerPort(Integer.toString(Integer.parseInt(config.getControllerPort())
+ controllerPortOffset++));
- ControllerStarter helixControllerStarter = new ControllerStarter(config2);
- helixControllerStarter.start();
- HelixManager helixControllerManager =
helixControllerStarter.getHelixControllerManager();
+ ControllerConf helixOnlyControllerConfig =
getDefaultControllerConfiguration();
+
helixOnlyControllerConfig.setControllerMode(ControllerConf.ControllerMode.HELIX_ONLY);
+
helixOnlyControllerConfig.setControllerPort(Integer.toString(DEFAULT_CONTROLLER_PORT
+ 1));
+ ControllerStarter helixOnlyController = new
ControllerStarter(helixOnlyControllerConfig);
+ helixOnlyController.start();
+ HelixManager helixControllerManager =
helixOnlyController.getHelixControllerManager();
HelixAdmin helixAdmin = helixControllerManager.getClusterManagmentTool();
TestUtils.waitForCondition(aVoid -> helixControllerManager.isConnected(),
TIMEOUT_IN_MS,
- "Failed to start " + config2.getControllerMode() + " controller in " +
TIMEOUT_IN_MS + "ms.");
-
- // Enable the lead controller resource.
- helixAdmin.enableResource(getHelixClusterName(),
CommonConstants.Helix.LEAD_CONTROLLER_RESOURCE_NAME, true);
+ "Failed to start the Helix-only controller");
// Starting a pinot only controller.
- ControllerConf config3 = getDefaultControllerConfiguration();
- config3.setHelixClusterName(getHelixClusterName());
- config3.setControllerMode(ControllerConf.ControllerMode.PINOT_ONLY);
-
config3.setControllerPort(Integer.toString(Integer.parseInt(config.getControllerPort())
+ controllerPortOffset++));
-
- ControllerStarter firstPinotOnlyController = new
TestOnlyControllerStarter(config3);
firstPinotOnlyController.start();
- PinotHelixResourceManager firstPinotOnlyPinotHelixResourceManager =
- firstPinotOnlyController.getHelixResourceManager();
+ PinotHelixResourceManager helixResourceManager =
firstPinotOnlyController.getHelixResourceManager();
+ TestUtils.waitForCondition(aVoid ->
helixResourceManager.getHelixZkManager().isConnected(), TIMEOUT_IN_MS,
+ "Failed to start the first Pinot-only controller");
- TestUtils.waitForCondition(aVoid ->
firstPinotOnlyPinotHelixResourceManager.getHelixZkManager().isConnected(),
- TIMEOUT_IN_MS, "Failed to start " + config.getControllerMode() + "
controller in " + TIMEOUT_IN_MS + "ms.");
- Assert.assertEquals(firstPinotOnlyController.getControllerMode(),
ControllerConf.ControllerMode.PINOT_ONLY);
+ // Enable the lead controller resource.
+ helixAdmin.enableResource(getHelixClusterName(),
LEAD_CONTROLLER_RESOURCE_NAME, true);
+ TestUtils.waitForCondition(aVoid -> {
+ ExternalView leadControllerResourceExternalView =
+ helixAdmin.getResourceExternalView(getHelixClusterName(),
LEAD_CONTROLLER_RESOURCE_NAME);
+ for (String partition :
leadControllerResourceExternalView.getPartitionSet()) {
+ Map<String, String> stateMap =
leadControllerResourceExternalView.getStateMap(partition);
+ if (stateMap.size() != 1 || stateMap.values().contains("MASTER")) {
+ return false;
+ }
+ }
+ return true;
+ }, TIMEOUT_IN_MS, "Failed to choose the only participant as MASTER");
// Start a second Pinot only controller.
- ControllerConf config4 = getDefaultControllerConfiguration();
- config4.setHelixClusterName(getHelixClusterName());
- config4.setControllerMode(ControllerConf.ControllerMode.PINOT_ONLY);
-
config4.setControllerPort(Integer.toString(Integer.parseInt(config.getControllerPort())
+ controllerPortOffset++));
-
- ControllerStarter secondControllerStarter = new
TestOnlyControllerStarter(config4);
- secondControllerStarter.start();
- // Two controller instances assigned to cluster.
- TestUtils
- .waitForCondition(aVoid ->
firstPinotOnlyPinotHelixResourceManager.getAllInstances().size() == 2,
TIMEOUT_IN_MS,
- "Failed to start the 2nd pinot only controller in " +
TIMEOUT_IN_MS + "ms.");
+ ControllerConf secondPinotOnlyControllerConfig =
getDefaultControllerConfiguration();
+
secondPinotOnlyControllerConfig.setControllerMode(ControllerConf.ControllerMode.PINOT_ONLY);
+
secondPinotOnlyControllerConfig.setControllerPort(Integer.toString(DEFAULT_CONTROLLER_PORT
+ 2));
+ ControllerStarter secondPinotOnlyController =
getControllerStarter(secondPinotOnlyControllerConfig);
+ secondPinotOnlyController.start();
+ TestUtils.waitForCondition(
+ aVoid ->
secondPinotOnlyController.getHelixResourceManager().getHelixZkManager().isConnected(),
TIMEOUT_IN_MS,
+ "Failed to start the second Pinot-only controller");
+ TestUtils.waitForCondition(aVoid -> {
+ ExternalView leadControllerResourceExternalView =
+ helixAdmin.getResourceExternalView(getHelixClusterName(),
LEAD_CONTROLLER_RESOURCE_NAME);
+ for (String partition :
leadControllerResourceExternalView.getPartitionSet()) {
+ Map<String, String> stateMap =
leadControllerResourceExternalView.getStateMap(partition);
+ if (stateMap.size() != 2 ||
stateMap.values().containsAll(Arrays.asList("MASTER", "SLAVE"))) {
Review comment:
Since the replica is set to 1, which means there'll be only 1 participant in
each of the partitions, the participant would ultimately be either MASTER or
OFFLINE. So there is no one in SLAVE state in the end.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
