You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2022/09/13 00:54:37 UTC
[pinot] branch master updated: Deprecate instanceId Config For Broker/Minion Specific Configs (#9308)
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/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 7f8ea6d182 Deprecate instanceId Config For Broker/Minion Specific Configs (#9308)
7f8ea6d182 is described below
commit 7f8ea6d182c418d64521ceb08a002df652c69bee
Author: Ankit Sultana <an...@uber.com>
AuthorDate: Tue Sep 13 06:24:31 2022 +0530
Deprecate instanceId Config For Broker/Minion Specific Configs (#9308)
---
.../broker/broker/helix/BaseBrokerStarter.java | 14 ++++++-----
.../broker/HelixBrokerStarterHostnamePortTest.java | 28 ++++++++++++++++++++++
.../java/org/apache/pinot/minion/MinionConf.java | 3 ++-
.../apache/pinot/spi/utils/CommonConstants.java | 4 +++-
.../pinot/tools/perf/PerfBenchmarkDriver.java | 2 +-
5 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
index 47cccc1cbb..268f5fda71 100644
--- a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
+++ b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
@@ -133,14 +133,16 @@ public abstract class BaseBrokerStarter implements ServiceStartable {
_port = _listenerConfigs.get(0).getPort();
_tlsPort = ListenerConfigUtil.findLastTlsPort(_listenerConfigs, -1);
- _instanceId = _brokerConf.getProperty(Helix.Instance.INSTANCE_ID_KEY);
- if (_instanceId != null) {
- // NOTE: Force all instances to have the same prefix in order to derive the instance type based on the instance id
- Preconditions.checkState(InstanceTypeUtils.isBroker(_instanceId), "Instance id must have prefix '%s', got '%s'",
- Helix.PREFIX_OF_BROKER_INSTANCE, _instanceId);
- } else {
+ _instanceId = _brokerConf.getProperty(Broker.CONFIG_OF_BROKER_ID);
+ if (_instanceId == null) {
+ _instanceId = _brokerConf.getProperty(Helix.Instance.INSTANCE_ID_KEY);
+ }
+ if (_instanceId == null) {
_instanceId = Helix.PREFIX_OF_BROKER_INSTANCE + _hostname + "_" + _port;
}
+ // NOTE: Force all instances to have the same prefix in order to derive the instance type based on the instance id
+ Preconditions.checkState(InstanceTypeUtils.isBroker(_instanceId), "Instance id must have prefix '%s', got '%s'",
+ Helix.PREFIX_OF_BROKER_INSTANCE, _instanceId);
_brokerConf.setProperty(Broker.CONFIG_OF_BROKER_ID, _instanceId);
}
diff --git a/pinot-broker/src/test/java/org/apache/pinot/broker/broker/HelixBrokerStarterHostnamePortTest.java b/pinot-broker/src/test/java/org/apache/pinot/broker/broker/HelixBrokerStarterHostnamePortTest.java
index 0d0e5d078f..d2e2d60140 100644
--- a/pinot-broker/src/test/java/org/apache/pinot/broker/broker/HelixBrokerStarterHostnamePortTest.java
+++ b/pinot-broker/src/test/java/org/apache/pinot/broker/broker/HelixBrokerStarterHostnamePortTest.java
@@ -30,6 +30,7 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import static org.apache.pinot.spi.utils.CommonConstants.Broker.CONFIG_OF_BROKER_HOSTNAME;
+import static org.apache.pinot.spi.utils.CommonConstants.Broker.CONFIG_OF_BROKER_ID;
import static org.apache.pinot.spi.utils.CommonConstants.Broker.CONFIG_OF_DELAY_SHUTDOWN_TIME_MS;
import static org.apache.pinot.spi.utils.CommonConstants.Helix.CONFIG_OF_CLUSTER_NAME;
import static org.apache.pinot.spi.utils.CommonConstants.Helix.CONFIG_OF_ZOOKEEPR_SERVER;
@@ -110,6 +111,33 @@ public class HelixBrokerStarterHostnamePortTest extends ControllerTest {
brokerStarter.stop();
}
+ @Test
+ public void testInstanceIdPrecedence()
+ throws Exception {
+ // Ensures that pinot.broker.instance.id has higher precedence compared to instanceId
+ Map<String, Object> properties = new HashMap<>();
+ properties.put(CONFIG_OF_ZOOKEEPR_SERVER, getZkUrl());
+ properties.put(CONFIG_OF_CLUSTER_NAME, getHelixClusterName());
+ properties.put(CONFIG_OF_BROKER_ID, "Broker_morePrecedence");
+ properties.put(INSTANCE_ID_KEY, "Broker_lessPrecedence");
+ properties.put(CONFIG_OF_BROKER_HOSTNAME, "myHost");
+ properties.put(KEY_OF_BROKER_QUERY_PORT, 1234);
+ properties.put(CONFIG_OF_DELAY_SHUTDOWN_TIME_MS, 0);
+
+ HelixBrokerStarter brokerStarter = new HelixBrokerStarter();
+ brokerStarter.init(new PinotConfiguration(properties));
+ brokerStarter.start();
+
+ String instanceId = brokerStarter.getInstanceId();
+ assertEquals(instanceId, "Broker_morePrecedence");
+ InstanceConfig instanceConfig = HelixHelper.getInstanceConfig(_helixManager, instanceId);
+ assertEquals(instanceConfig.getInstanceName(), instanceId);
+ assertEquals(instanceConfig.getHostName(), "myHost");
+ assertEquals(instanceConfig.getPort(), "1234");
+
+ brokerStarter.stop();
+ }
+
@AfterClass
public void tearDown() {
stopController();
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
index 0981612273..282457901b 100644
--- a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
+++ b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
@@ -58,7 +58,8 @@ public class MinionConf extends PinotConfiguration {
}
public String getInstanceId() {
- return getProperty(CommonConstants.Helix.Instance.INSTANCE_ID_KEY);
+ String instanceId = getProperty(CommonConstants.Minion.CONFIG_OF_MINION_ID);
+ return instanceId != null ? instanceId : getProperty(CommonConstants.Helix.Instance.INSTANCE_ID_KEY);
}
public int getEndReplaceSegmentsTimeoutMs() {
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index 5b67b47217..a1007953a6 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -136,6 +136,7 @@ public class CommonConstants {
}
public static class Instance {
+ @Deprecated
public static final String INSTANCE_ID_KEY = "instanceId";
public static final String DATA_DIR_KEY = "dataDir";
public static final String ADMIN_PORT_KEY = "adminPort";
@@ -210,7 +211,7 @@ public class CommonConstants {
public static final double DEFAULT_BROKER_QUERY_LOG_MAX_RATE_PER_SECOND = 10_000d;
public static final String CONFIG_OF_BROKER_TIMEOUT_MS = "pinot.broker.timeoutMs";
public static final long DEFAULT_BROKER_TIMEOUT_MS = 10_000L;
- public static final String CONFIG_OF_BROKER_ID = "pinot.broker.id";
+ public static final String CONFIG_OF_BROKER_ID = "pinot.broker.instance.id";
public static final String CONFIG_OF_BROKER_HOSTNAME = "pinot.broker.hostname";
public static final String CONFIG_OF_SWAGGER_USE_HTTPS = "pinot.broker.swagger.use.https";
// Configuration to consider the broker ServiceStatus as being STARTED if the percent of resources (tables) that
@@ -543,6 +544,7 @@ public class CommonConstants {
public static class Minion {
public static final String CONFIG_OF_METRICS_PREFIX = "pinot.minion.";
+ public static final String CONFIG_OF_MINION_ID = "pinot.minion.instance.id";
public static final String METADATA_EVENT_OBSERVER_PREFIX = "metadata.event.notifier";
// Config keys
diff --git a/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java b/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
index b920c8a1d3..c8b4020891 100644
--- a/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
+++ b/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
@@ -240,7 +240,7 @@ public class PerfBenchmarkDriver {
String brokerInstanceName = "Broker_localhost_" + CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT;
Map<String, Object> properties = new HashMap<>();
- properties.put(CommonConstants.Helix.Instance.INSTANCE_ID_KEY, brokerInstanceName);
+ properties.put(CommonConstants.Broker.CONFIG_OF_BROKER_ID, brokerInstanceName);
properties.put(CommonConstants.Broker.CONFIG_OF_BROKER_TIMEOUT_MS, BROKER_TIMEOUT_MS);
properties.put(CommonConstants.Helix.CONFIG_OF_CLUSTER_NAME, _clusterName);
properties.put(CommonConstants.Helix.CONFIG_OF_ZOOKEEPR_SERVER, _zkAddress);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org