You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by mc...@apache.org on 2021/06/04 00:33:49 UTC
[incubator-pinot] branch master updated: Resolve bug in
HelixServerStarter for populating environment property map field and remame
timeout variables to follow timeunit convention (#7019)
This is an automated email from the ASF dual-hosted git repository.
mcvsubbu 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 1d1bf4d Resolve bug in HelixServerStarter for populating environment property map field and remame timeout variables to follow timeunit convention (#7019)
1d1bf4d is described below
commit 1d1bf4d96785a1011066f6ee897fc7985b31686c
Author: Jay Desai <ja...@gmail.com>
AuthorDate: Thu Jun 3 17:33:22 2021 -0700
Resolve bug in HelixServerStarter for populating environment property map field and remame timeout variables to follow timeunit convention (#7019)
Co-authored-by: Jay Desai <ja...@jadesai-mn2.linkedin.biz>
---
.../pinot/plugin/provider/AzureEnvironmentProvider.java | 16 ++++++----------
.../pinot/server/starter/helix/HelixServerStarter.java | 2 +-
.../PinotEnvironmentProviderFactoryTest.java | 8 ++++----
3 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/pinot-plugins/pinot-environment/pinot-azure/src/main/java/org/apache/pinot/plugin/provider/AzureEnvironmentProvider.java b/pinot-plugins/pinot-environment/pinot-azure/src/main/java/org/apache/pinot/plugin/provider/AzureEnvironmentProvider.java
index c710ae8..c9c59d5 100644
--- a/pinot-plugins/pinot-environment/pinot-azure/src/main/java/org/apache/pinot/plugin/provider/AzureEnvironmentProvider.java
+++ b/pinot-plugins/pinot-environment/pinot-azure/src/main/java/org/apache/pinot/plugin/provider/AzureEnvironmentProvider.java
@@ -40,8 +40,6 @@ import org.apache.http.impl.client.HttpClients;
import org.apache.http.util.EntityUtils;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.environmentprovider.PinotEnvironmentProvider;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
@@ -49,12 +47,10 @@ import org.slf4j.LoggerFactory;
*/
public class AzureEnvironmentProvider implements PinotEnvironmentProvider {
- private static final Logger LOGGER = LoggerFactory.getLogger(AzureEnvironmentProvider.class);
-
protected static final String MAX_RETRY = "maxRetry";
protected static final String IMDS_ENDPOINT = "imdsEndpoint";
- protected static final String CONNECTION_TIMEOUT = "connectionTimeout";
- protected static final String REQUEST_TIMEOUT = "requestTimeout";
+ protected static final String CONNECTION_TIMEOUT_MILLIS = "connectionTimeoutMillis";
+ protected static final String REQUEST_TIMEOUT_MILLIS = "requestTimeoutMillis";
private static final String COMPUTE = "compute";
private static final String METADATA = "Metadata";
private static final String PLATFORM_FAULT_DOMAIN = "platformFaultDomain";
@@ -73,12 +69,12 @@ public class AzureEnvironmentProvider implements PinotEnvironmentProvider {
_maxRetry = Integer.parseInt(pinotConfiguration.getProperty(MAX_RETRY));
_imdsEndpoint = pinotConfiguration.getProperty(IMDS_ENDPOINT);
- int connectionTimeout = Integer.parseInt(pinotConfiguration.getProperty(CONNECTION_TIMEOUT));
- int requestTimeout = Integer.parseInt(pinotConfiguration.getProperty(REQUEST_TIMEOUT));
+ int connectionTimeoutMillis = Integer.parseInt(pinotConfiguration.getProperty(CONNECTION_TIMEOUT_MILLIS));
+ int requestTimeoutMillis = Integer.parseInt(pinotConfiguration.getProperty(REQUEST_TIMEOUT_MILLIS));
final RequestConfig requestConfig = RequestConfig.custom()
- .setConnectTimeout(connectionTimeout)
- .setConnectionRequestTimeout(requestTimeout)
+ .setConnectTimeout(connectionTimeoutMillis)
+ .setConnectionRequestTimeout(requestTimeoutMillis)
.build();
final HttpRequestRetryHandler httpRequestRetryHandler = (iOException, executionCount, httpContext) ->
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 1a154e4..ef239fe 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
@@ -303,7 +303,7 @@ public class HelixServerStarter implements ServiceStartable {
Map<String, String> existingEnvironmentConfigsMap = instanceConfig.getRecord().getMapField(
CommonConstants.ENVIRONMENT_IDENTIFIER);
- if (existingEnvironmentConfigsMap != null && !existingEnvironmentConfigsMap.equals(environmentProperties)) {
+ if (existingEnvironmentConfigsMap == null || !existingEnvironmentConfigsMap.equals(environmentProperties)) {
instanceConfig.getRecord().setMapField(CommonConstants.ENVIRONMENT_IDENTIFIER, environmentProperties);
LOGGER.info("Adding environment properties: {} for instance: {}", environmentProperties, _instanceId);
needToUpdateInstanceConfig = true;
diff --git a/pinot-spi/src/test/java/org/apache/pinot/spi/environmentprovider/PinotEnvironmentProviderFactoryTest.java b/pinot-spi/src/test/java/org/apache/pinot/spi/environmentprovider/PinotEnvironmentProviderFactoryTest.java
index c6b54fb..b0ab4f4 100644
--- a/pinot-spi/src/test/java/org/apache/pinot/spi/environmentprovider/PinotEnvironmentProviderFactoryTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/environmentprovider/PinotEnvironmentProviderFactoryTest.java
@@ -31,8 +31,8 @@ public class PinotEnvironmentProviderFactoryTest {
Map<String, Object> properties = new HashMap<>();
properties.put("class.test", PinotEnvironmentProviderFactoryTest.TestEnvironmentProvider.class.getName());
properties.put("test.maxRetry", "3");
- properties.put("test.connectionTimeout", "100");
- properties.put("test.requestTimeout", "100");
+ properties.put("test.connectionTimeoutMillis", "100");
+ properties.put("test.requestTimeoutMillis", "100");
PinotEnvironmentProviderFactory.init(new PinotConfiguration(properties));
PinotEnvironmentProvider testPinotEnvironment = PinotEnvironmentProviderFactory.getEnvironmentProvider("test");
@@ -42,9 +42,9 @@ public class PinotEnvironmentProviderFactoryTest {
Assert.assertEquals(((PinotEnvironmentProviderFactoryTest.TestEnvironmentProvider)
testPinotEnvironment).getConfiguration().getProperty("maxRetry"), "3");
Assert.assertEquals(((PinotEnvironmentProviderFactoryTest.TestEnvironmentProvider)
- testPinotEnvironment).getConfiguration().getProperty("connectionTimeout"), "100");
+ testPinotEnvironment).getConfiguration().getProperty("connectionTimeoutMillis"), "100");
Assert.assertEquals(((PinotEnvironmentProviderFactoryTest.TestEnvironmentProvider)
- testPinotEnvironment).getConfiguration().getProperty("requestTimeout"), "100");
+ testPinotEnvironment).getConfiguration().getProperty("requestTimeoutMillis"), "100");
}
public static class TestEnvironmentProvider implements PinotEnvironmentProvider {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org