This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 6a97127 Decouple server instance id with hostname/port config. (#4995)
6a97127 is described below
commit 6a9712728a1f0e981f9726573533633579ccd35b
Author: Ting Chen <[email protected]>
AuthorDate: Wed Jan 29 09:44:54 2020 -0800
Decouple server instance id with hostname/port config. (#4995)
Currently Helix will derived hostname/port from the server instanceId
string (as demonstrated in the test _testDisableLogicalServerID()_) if the
instance id is set in server config. This PR decouples the configuration of
server instance id and hostname/port. This allows Pinot user to set the
following field independently:
- Instance ID
- server_netty_host
- server_netty_port
This is done by overwriting the Helix zk data after server connects to the
zk cluster. It is hidden behind a flag now so the current use cases will not be
affected.
Also added an integration test to test different configuration choices for
servers.
---
.../tests/ServerStarterIntegrationTest.java | 148 +++++++++++++++++++++
.../server/starter/helix/HelixServerStarter.java | 51 ++++++-
2 files changed, 194 insertions(+), 5 deletions(-)
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ServerStarterIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ServerStarterIntegrationTest.java
new file mode 100644
index 0000000..2463876
--- /dev/null
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ServerStarterIntegrationTest.java
@@ -0,0 +1,148 @@
+/**
+ * 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.
+ */
+package org.apache.pinot.integration.tests;
+
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.ZkStarter;
+import org.apache.pinot.controller.helix.ControllerTest;
+import org.apache.pinot.server.starter.helix.HelixServerStarter;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.KEY_OF_SERVER_NETTY_HOST;
+import static
org.apache.pinot.common.utils.CommonConstants.Helix.KEY_OF_SERVER_NETTY_PORT;
+import static
org.apache.pinot.common.utils.CommonConstants.Server.CONFIG_OF_INSTANCE_ID;
+import static org.testng.Assert.assertEquals;
+
+
+public class ServerStarterIntegrationTest extends ControllerTest {
+ public static final String SERVER1 = "Server1";
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ startZk();
+ startController();
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+ stopController();
+ stopZk();
+ }
+
+ private void verifyZkConfigData(HelixServerStarter helixServerStarter,
String expectedInstanceName,
+ String expectedHostname, String expectedPort) {
+ // Verify the serverId, host and port are set correctly in Zk.
+ HelixManager helixManager = helixServerStarter.getHelixManager();
+ PropertyKey.Builder keyBuilder =
helixManager.getHelixDataAccessor().keyBuilder();
+ InstanceConfig config = helixManager.getHelixDataAccessor().
+
getProperty(keyBuilder.instanceConfig(helixServerStarter.getHelixManager().getInstanceName()));
+ helixServerStarter.stop();
+
+ assertEquals(config.getInstanceName(), expectedInstanceName);
+ // By default (auto joined instances), server instance name is of format:
{@code Server_<hostname>_<port>}, e.g.
+ // {@code Server_localhost_12345}, hostname is of format: {@code
Server_<hostname>}, e.g. {@code Server_localhost}.
+ // More details refer to the class ServerInstance.
+ assertEquals(config.getHostName(), expectedHostname);
+ assertEquals(config.getPort(), expectedPort);
+ }
+
+ @Test
+ public void testWithNoInstanceIdNoHostnamePort()
+ throws Exception {
+ // Test the behavior when no instance id nor hostname/port is specified in
server conf.
+ Configuration serverConf = new PropertiesConfiguration();
+ // Start the server.
+ HelixServerStarter helixServerStarter =
+ new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+ String expectedInstanceName =
helixServerStarter.getHelixManager().getInstanceName();
+ String expectedHostname = expectedInstanceName.substring(0,
expectedInstanceName.lastIndexOf('_'));
+ verifyZkConfigData(helixServerStarter, expectedInstanceName,
expectedHostname, "8098");
+ }
+
+ @Test
+ public void testWithNoInstanceIdButWithHostnamePort()
+ throws Exception {
+ // Test the behavior when no instance id specified but hostname/port is
specified in server conf.
+ Configuration serverConf = new PropertiesConfiguration();
+ serverConf.setProperty(KEY_OF_SERVER_NETTY_HOST, "host1");
+ serverConf.setProperty(KEY_OF_SERVER_NETTY_PORT, 10001);
+ // Start the server
+ HelixServerStarter helixServerStarter =
+ new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+
+ // Verify the serverId, host and port are set correctly in Zk.
+ verifyZkConfigData(helixServerStarter, "Server_host1_10001",
"Server_host1", "10001");
+ }
+
+ @Test
+ public void testInstanceIdWithHostPortInfo()
+ throws Exception {
+ // Test the behavior when logical server id and hostname/port are all
specified in server conf.
+ // Unlike testInstanceIdOnly(), the host and port info will be overwritten
with those in config.
+ Configuration serverConf = new PropertiesConfiguration();
+ serverConf.setProperty(CONFIG_OF_INSTANCE_ID, SERVER1);
+ serverConf.setProperty(KEY_OF_SERVER_NETTY_HOST, "host1");
+ serverConf.setProperty(KEY_OF_SERVER_NETTY_PORT, 10001);
+ // Start the server
+ HelixServerStarter helixServerStarter =
+ new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+
+ // Verify the serverId, host and port are set correctly in Zk.
+ verifyZkConfigData(helixServerStarter, SERVER1, "host1", "10001");
+ }
+
+ @Test
+ public void testInstanceIdOnly()
+ throws Exception {
+ Configuration serverConf = new PropertiesConfiguration();
+ serverConf.setProperty(CONFIG_OF_INSTANCE_ID, SERVER1);
+ // Start the server
+ HelixServerStarter helixServerStarter =
+ new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+
+ // Verify the serverId, host and port are set correctly in Zk.
+ // Note that helix will use INSTANCE_ID to extract the hostname instead of
using netty host/port in config.
+ verifyZkConfigData(helixServerStarter, SERVER1, SERVER1, "");
+ }
+
+ @Test
+ public void testSetInstanceIdToHostname()
+ throws Exception {
+ // Test the behavior when no instance id nor hostname/port is specified in
server conf.
+ // Compared with testWithNoInstanceIdNoHostnamePort(), here the server id
use hostname (i.e., localhost) instead of
+ // host address (i.e., 127.0.0.1).
+ Configuration serverConf = new PropertiesConfiguration();
+
serverConf.setProperty(CommonConstants.Helix.SET_INSTANCE_ID_TO_HOSTNAME_KEY,
true);
+ // Start the server
+ HelixServerStarter helixServerStarter =
+ new HelixServerStarter(getHelixClusterName(),
ZkStarter.DEFAULT_ZK_STR, serverConf);
+
+ // Verify the serverId, host and port are set correctly in Zk.
+ verifyZkConfigData(helixServerStarter, "Server_localhost_8098",
"Server_localhost", "8098");
+ }
+}
diff --git
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
index de9cec6..a0816e1 100644
---
a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
+++
b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
@@ -31,6 +31,7 @@ import org.apache.commons.configuration.BaseConfiguration;
import org.apache.commons.configuration.Configuration;
import org.apache.commons.configuration.ConfigurationUtils;
import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixException;
import org.apache.helix.HelixManager;
import org.apache.helix.HelixManagerFactory;
import org.apache.helix.InstanceType;
@@ -146,7 +147,7 @@ public class HelixServerStarter {
LOGGER.info("Connecting Helix manager");
_helixManager.connect();
_helixAdmin = _helixManager.getClusterManagmentTool();
- addInstanceTagIfNeeded(helixClusterName, _instanceId);
+ updateInstanceConfigIfNeeded(helixClusterName, _instanceId, serverConf);
// Start restlet server for admin API endpoint
int adminApiPort = _serverConf.getInt(CONFIG_OF_ADMIN_API_PORT,
DEFAULT_ADMIN_API_PORT);
@@ -266,16 +267,52 @@ public class HelixServerStarter {
_helixAdmin.setConfig(scope, props);
}
- private void addInstanceTagIfNeeded(String clusterName, String instanceName)
{
+ private void updateInstanceConfigIfNeeded(String clusterName, String
instanceName, Configuration serverConf) {
InstanceConfig instanceConfig = _helixAdmin.getInstanceConfig(clusterName,
instanceName);
List<String> instanceTags = instanceConfig.getTags();
+ boolean toUpdateHelixRecord = false;
if (instanceTags == null || instanceTags.size() == 0) {
if
(ZKMetadataProvider.getClusterTenantIsolationEnabled(_helixManager.getHelixPropertyStore()))
{
- _helixAdmin.addInstanceTag(clusterName, instanceName,
TagNameUtils.getOfflineTagForTenant(null));
- _helixAdmin.addInstanceTag(clusterName, instanceName,
TagNameUtils.getRealtimeTagForTenant(null));
+ instanceConfig.addTag(TagNameUtils.getOfflineTagForTenant(null));
+ instanceConfig.addTag(TagNameUtils.getRealtimeTagForTenant(null));
} else {
- _helixAdmin.addInstanceTag(clusterName, instanceName,
UNTAGGED_SERVER_INSTANCE);
+ instanceConfig.addTag(UNTAGGED_SERVER_INSTANCE);
}
+ toUpdateHelixRecord = true;
+ }
+
+ // If the server config has both instance_id and host/port info, overwrite
the host/port info in zk. Without the
+ // overwrite, Helix will extract host/port from the instance_id instead of
use those in config.
+ // Use serverConf instead of _serverConf as the latter has been modified.
+ if (serverConf.containsKey(CONFIG_OF_INSTANCE_ID)) {
+ // Internally, Helix use instanceId to derive Hostname and Port. To
decouple them, explicitly set the hostname/port
+ // field in zk.
+ String hostName = serverConf.getString(KEY_OF_SERVER_NETTY_HOST);
+ if (hostName != null && !hostName.equals(instanceConfig.getHostName())) {
+ instanceConfig.setHostName(hostName);
+ toUpdateHelixRecord = true;
+ }
+
+ if (serverConf.containsKey(KEY_OF_SERVER_NETTY_PORT)) {
+ String portStr =
Integer.toString(serverConf.getInt(KEY_OF_SERVER_NETTY_PORT));
+ if (portStr != null && !portStr.equals(instanceConfig.getPort())) {
+ instanceConfig.setPort(portStr);
+ toUpdateHelixRecord = true;
+ }
+ }
+ }
+ if (!toUpdateHelixRecord) {
+ return;
+ }
+ // Use setProperty instead of _helixAdmin.setInstanceConfig because the
latter explicitly forbids instance host
+ // port modification.
+ if(_helixManager.getHelixDataAccessor().setProperty(
+
_helixManager.getHelixDataAccessor().keyBuilder().instanceConfig(instanceName),
instanceConfig)) {
+ LOGGER.info("Updated server hostname/port successfully for server id {}
to {}:", instanceName, instanceConfig);
+ } else {
+ LOGGER.error("Failed to update hostname/port for instance: {}",
instanceName);
+ // Treat this is as a fatal error.
+ throw new HelixException("Failed to update hostname/port for instance "
+ instanceName);
}
}
@@ -509,6 +546,10 @@ public class HelixServerStarter {
return true;
}
+ public HelixManager getHelixManager() {
+ return _helixManager;
+ }
+
/**
* This method is for reference purpose only.
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
