You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ro...@apache.org on 2022/07/14 16:34:18 UTC
[pinot] branch master updated: BUGFIX: Adding util for getting URL from InstanceConfig (#8856)
This is an automated email from the ASF dual-hosted git repository.
rongr 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 572e9628ad BUGFIX: Adding util for getting URL from InstanceConfig (#8856)
572e9628ad is described below
commit 572e9628ad12202e744a4511fd3dbd9c9f3f68d8
Author: Tim Santos <ti...@cortexdata.io>
AuthorDate: Thu Jul 14 09:34:14 2022 -0700
BUGFIX: Adding util for getting URL from InstanceConfig (#8856)
* Adding method for getting a components URL from InstanceConfig. Also changing auto-tuner interface
to take http headers.
* Remove backwards compatibility check for hostnames
* Adding null check handling
* Adding test
* Update util javadoc to be more clear
---
.../pinot/common/helix/ExtraInstanceConfig.java | 29 ++++++++++++++++++++++
.../tuner/NoOpTableTableConfigTuner.java | 6 +++++
.../controller/tuner/RealTimeAutoIndexTuner.java | 6 +++++
.../pinot/controller/tuner/TableConfigTuner.java | 12 +++++++++
.../core/query/executor/sql/SqlQueryExecutor.java | 23 +++++++++++------
.../integration/tests/TlsIntegrationTest.java | 11 ++++++++
6 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/helix/ExtraInstanceConfig.java b/pinot-common/src/main/java/org/apache/pinot/common/helix/ExtraInstanceConfig.java
index 6982093db0..259eb1cf1f 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/helix/ExtraInstanceConfig.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/helix/ExtraInstanceConfig.java
@@ -20,6 +20,7 @@
package org.apache.pinot.common.helix;
import org.apache.helix.model.InstanceConfig;
+import org.apache.pinot.spi.utils.CommonConstants;
/**
@@ -44,4 +45,32 @@ public class ExtraInstanceConfig {
public void setTlsPort(String tlsPort) {
_proxy.getRecord().setSimpleField(PinotInstanceConfigProperty.PINOT_TLS_PORT.toString(), tlsPort);
}
+
+ /**
+ * Returns an instance URL from the InstanceConfig. Will set the appropriate protocol and port. Note that the helix
+ * participant port will be returned. For the Pinot server this will not correspond to the admin port.
+ * Returns null if the URL cannot be constructed.
+ */
+ public String getComponentUrl() {
+ String protocol = null;
+ String port = null;
+ try {
+ if (Integer.parseInt(getTlsPort()) > 0) {
+ protocol = CommonConstants.HTTPS_PROTOCOL;
+ port = getTlsPort();
+ }
+ } catch (Exception e) {
+ try {
+ if (Integer.parseInt(_proxy.getPort()) > 0) {
+ protocol = CommonConstants.HTTP_PROTOCOL;
+ port = _proxy.getPort();
+ }
+ } catch (Exception ignored) {
+ }
+ }
+ if (port != null) {
+ return String.format("%s://%s:%s", protocol, _proxy.getHostName(), port);
+ }
+ return null;
+ }
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
index f8e18a7469..92f28fe64a 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
@@ -32,4 +32,10 @@ public class NoOpTableTableConfigTuner implements TableConfigTuner {
TableConfig initialConfig, Schema schema, Map<String, String> extraProperties) {
return initialConfig;
}
+
+ @Override
+ public TableConfig apply(PinotHelixResourceManager pinotHelixResourceManager,
+ TableConfig initialConfig, Schema schema, Map<String, String> extraProperties, Map<String, String> httpHeaders) {
+ return initialConfig;
+ }
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
index a1d25eb790..28e1ab6081 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
@@ -42,4 +42,10 @@ public class RealTimeAutoIndexTuner implements TableConfigTuner {
initialIndexingConfig.setNoDictionaryColumns(schema.getMetricNames());
return tableConfig;
}
+
+ @Override
+ public TableConfig apply(PinotHelixResourceManager pinotHelixResourceManager,
+ TableConfig tableConfig, Schema schema, Map<String, String> extraProperties, Map<String, String> httpHeaders) {
+ return apply(pinotHelixResourceManager, tableConfig, schema, extraProperties);
+ }
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
index 3f84ce163d..a75340706b 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
@@ -42,4 +42,16 @@ public interface TableConfigTuner {
*/
TableConfig apply(@Nullable PinotHelixResourceManager pinotHelixResourceManager,
TableConfig tableConfig, Schema schema, Map<String, String> extraProperties);
+
+ /**
+ * Apply tuner to a {@link TableConfig}.
+ *
+ * @param pinotHelixResourceManager Pinot Helix Resource Manager to access Helix resources
+ * @param tableConfig tableConfig that needs to be tuned.
+ * @param schema Table schema
+ * @param extraProperties extraProperties for the tuner implementation.
+ * @param httpHeaders Http headers required for any remote calls
+ */
+ TableConfig apply(@Nullable PinotHelixResourceManager pinotHelixResourceManager,
+ TableConfig tableConfig, Schema schema, Map<String, String> extraProperties, Map<String, String> httpHeaders);
}
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/sql/SqlQueryExecutor.java b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/sql/SqlQueryExecutor.java
index 19c4b10a6e..9d7239203a 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/sql/SqlQueryExecutor.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/sql/SqlQueryExecutor.java
@@ -24,14 +24,18 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
+import org.apache.helix.HelixDataAccessor;
import org.apache.helix.HelixManager;
+import org.apache.helix.PropertyKey;
import org.apache.pinot.common.exception.QueryException;
+import org.apache.pinot.common.helix.ExtraInstanceConfig;
import org.apache.pinot.common.minion.MinionClient;
import org.apache.pinot.common.response.BrokerResponse;
import org.apache.pinot.common.response.broker.BrokerResponseNative;
import org.apache.pinot.common.response.broker.ResultTable;
import org.apache.pinot.common.utils.helix.LeadControllerUtils;
import org.apache.pinot.spi.config.task.AdhocTaskConfig;
+import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.sql.parsers.SqlNodeAndOptions;
import org.apache.pinot.sql.parsers.dml.DataManipulationStatement;
import org.apache.pinot.sql.parsers.dml.DataManipulationStatementParser;
@@ -64,17 +68,20 @@ public class SqlQueryExecutor {
}
private static String getControllerBaseUrl(HelixManager helixManager) {
- String instanceHostPort = LeadControllerUtils.getHelixClusterLeader(helixManager);
- if (instanceHostPort == null) {
+ String instanceId = LeadControllerUtils.getHelixClusterLeader(helixManager);
+ if (instanceId == null) {
throw new RuntimeException("Unable to locate the leader pinot controller, please retry later...");
}
- int index = instanceHostPort.lastIndexOf('_');
- if (index < 0) {
- throw new RuntimeException("Unable to parse pinot controller instance name for " + instanceHostPort);
+
+ HelixDataAccessor helixDataAccessor = helixManager.getHelixDataAccessor();
+ PropertyKey.Builder keyBuilder = helixDataAccessor.keyBuilder();
+ ExtraInstanceConfig extraInstanceConfig = new ExtraInstanceConfig(helixDataAccessor.getProperty(
+ keyBuilder.instanceConfig(CommonConstants.Helix.PREFIX_OF_CONTROLLER_INSTANCE + instanceId)));
+ String controllerBaseUrl = extraInstanceConfig.getComponentUrl();
+ if (controllerBaseUrl == null) {
+ throw new RuntimeException("Unable to extract the base url from the leader pinot controller");
}
- String leaderHost = instanceHostPort.substring(0, index);
- String leaderPort = instanceHostPort.substring(index + 1);
- return "http://" + leaderHost + ":" + leaderPort;
+ return controllerBaseUrl;
}
/**
diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java
index df26a61641..7d21ee7c48 100644
--- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java
+++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java
@@ -30,6 +30,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Properties;
import java.util.stream.Collectors;
import org.apache.commons.configuration.ConfigurationException;
@@ -527,6 +528,16 @@ public class TlsIntegrationTest extends BaseClusterIntegrationTest {
}
}
+ @Test
+ public void testComponentUrlWithTlsPort() {
+ List<InstanceConfig> instanceConfigs = HelixHelper.getInstanceConfigs(_helixManager);
+ List<String> httpsComponentUrls = instanceConfigs.stream().map(ExtraInstanceConfig::new)
+ .filter(pinotInstanceConfig -> pinotInstanceConfig.getTlsPort() != null)
+ .map(ExtraInstanceConfig::getComponentUrl).filter(Objects::nonNull).collect(Collectors.toList());
+
+ Assert.assertFalse(httpsComponentUrls.isEmpty());
+ }
+
private java.sql.Connection getValidJDBCConnection(int controllerPort) throws Exception {
SSLContextBuilder sslContextBuilder = SSLContextBuilder.create();
sslContextBuilder.setKeyStoreType(PKCS_12);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org