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_r286085295
##########
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
##########
@@ -337,12 +237,47 @@ private void setupHelixSystemProperties() {
// NOTE: Helix will disconnect the manager and disable the instance if it
detects flapping (too frequent disconnect
// from ZooKeeper). Setting flapping time window to a small value can
avoid this from happening. Helix ignores the
// non-positive value, so set the default value as 1.
- System.setProperty(SystemPropertyKeys.FLAPPING_TIME_WINDOW,
_helixServerConfig
-
.getString(CommonConstants.Helix.CONFIG_OF_SERVER_FLAPPING_TIME_WINDOW_MS,
- CommonConstants.Helix.DEFAULT_FLAPPING_TIME_WINDOW_MS));
+ System.setProperty(SystemPropertyKeys.FLAPPING_TIME_WINDOW,
+ _serverConf.getString(CONFIG_OF_SERVER_FLAPPING_TIME_WINDOW_MS,
DEFAULT_FLAPPING_TIME_WINDOW_MS));
+ }
+
+ /**
+ * When the server starts, check if the service status turns GOOD.
+ *
+ * @param endTimeMs Timeout for the check
+ */
+ private void startupServiceStatusCheck(long endTimeMs) {
+ LOGGER.info("Starting startup service status check");
+ long startTimeMs = System.currentTimeMillis();
+ long checkIntervalMs = _serverConf
+ .getLong(CONFIG_OF_STARTUP_SERVICE_STATUS_CHECK_INTERVAL_MS,
DEFAULT_STARTUP_SERVICE_STATUS_CHECK_INTERVAL_MS);
+
+ while (System.currentTimeMillis() < endTimeMs) {
+ Status serviceStatus = ServiceStatus.getServiceStatus();
Review comment:
This can be considered as an enhancement and addressed later; ServiceStatus
exposes polling based semantics; with this change it can be called by multiple
'actors'; one is the server that is starting up by itself, and external deploy
tools calling health check. It would be cleaner if the status check can be
turned into a something that runs in a thread and updates the state. All other
callers conditionally-wait on that particular state instead of triggering the
check yet again.
----------------------------------------------------------------
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
