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

Reply via email to