[GitHub] [incubator-pinot] sunithabeeram commented on a change in pull request #4222: Add startup/shutdown checks for HelixServerStarter

[GitBox <gi...@apache.org>]

sunithabeeram commented on a change in pull request #4222: Add startup/shutdown checks for HelixServerStarter
URL: https://github.com/apache/incubator-pinot/pull/4222#discussion_r286076909
 
 

 ##########
 File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
 ##########
 @@ -72,61 +63,66 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.pinot.common.utils.CommonConstants.Helix.*;
+import static org.apache.pinot.common.utils.CommonConstants.Server.*;
+
 
 /**
- * Single server helix starter. Will start automatically with an untagged box.
- * Will auto join current cluster as a participant.
- *
- *
- *
+ * Starter for Helix-based Pinot server.
+ * <p>When an instance starts for the first time, it will automatically join the Helix cluster with the default tag.
+ * <ul>
+ *   <li>
+ *     Optional start-up checks:
+ *     <ul>
+ *       <li>Service status check (ON by default)</li>
+ *     </ul>
+ *   </li>
+ *   <li>
+ *     Optional shut-down checks:
+ *     <ul>
+ *       <li>Query check (drains and finishes existing queries, ON by default)</li>
+ *       <li>Resource check (wait for all resources OFFLINE, OFF by default)</li>
+ *     </ul>
+ *   </li>
+ * </ul>
  */
 public class HelixServerStarter {
   private static final Logger LOGGER = LoggerFactory.getLogger(HelixServerStarter.class);
 
   private final String _helixClusterName;
-  private final Configuration _helixServerConfig;
+  private final Configuration _serverConf;
   private final String _instanceId;
-  private final long _maxQueryTimeMs;
-  private final long _maxShutdownWaitTimeMs;
-  private final long _checkIntervalTimeMs;
   private final HelixManager _helixManager;
   private final HelixAdmin _helixAdmin;
   private final ServerInstance _serverInstance;
   private final AdminApiApplication _adminApiApplication;
   private final String _zkServers;
 
-  public HelixServerStarter(String helixClusterName, String zkServer, Configuration helixServerConfig)
+  public HelixServerStarter(String helixClusterName, String zkServer, Configuration serverConf)
       throws Exception {
     LOGGER.info("Starting Pinot server");
+    long startTimeMs = System.currentTimeMillis();
     _helixClusterName = helixClusterName;
 
     // Make a clone so that changes to the config won't propagate to the caller
-    _helixServerConfig = ConfigurationUtils.cloneConfiguration(helixServerConfig);
-
-    if (_helixServerConfig.containsKey(CommonConstants.Server.CONFIG_OF_INSTANCE_ID)) {
-      _instanceId = _helixServerConfig.getString(CommonConstants.Server.CONFIG_OF_INSTANCE_ID);
-    } else {
-      String host =
-          _helixServerConfig.getString(CommonConstants.Helix.KEY_OF_SERVER_NETTY_HOST, NetUtil.getHostAddress());
-      int port = _helixServerConfig
-          .getInt(CommonConstants.Helix.KEY_OF_SERVER_NETTY_PORT, CommonConstants.Helix.DEFAULT_SERVER_NETTY_PORT);
-      _instanceId = CommonConstants.Helix.PREFIX_OF_SERVER_INSTANCE + host + "_" + port;
-      _helixServerConfig.addProperty(CommonConstants.Server.CONFIG_OF_INSTANCE_ID, _instanceId);
+    _serverConf = ConfigurationUtils.cloneConfiguration(serverConf);
+
+    // Log warnings for usage of deprecated config keys
+    String[] deprecatedConfigKeys =
+        new String[]{CONFIG_OF_STARTER_ENABLE_SEGMENTS_LOADING_CHECK, CONFIG_OF_STARTER_TIMEOUT_IN_SECONDS, CONFIG_OF_ENABLE_SHUTDOWN_DELAY, CONFIG_OF_INSTANCE_MAX_SHUTDOWN_WAIT_TIME, CONFIG_OF_INSTANCE_CHECK_INTERVAL_TIME};
+    for (String deprecatedConfigKey : deprecatedConfigKeys) {
+      if (_serverConf.containsKey(deprecatedConfigKey)) {
+        LOGGER.warn("Found usage of deprecated key: {}", deprecatedConfigKey);
 
 Review comment:
   Might be good to indicate what this is replaced with.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org