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 <jaydesai...@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 <jade...@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-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org