mcvsubbu commented on a change in pull request #4222: Add startup/shutdown checks for HelixServerStarter URL: https://github.com/apache/incubator-pinot/pull/4222#discussion_r286244988
########## File path: pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java ########## @@ -72,61 +63,67 @@ 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 Pinot server. + * <p>When the server 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 config key: {}, check CommonConstants for the new config keys", Review comment: I would keep a map of the old and new key (or, a map of old key and error message to be displayed). Try not to ask user to refer to source code. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org