snleee commented on a change in pull request #4222: Add startup/shutdown checks
for HelixServerStarter
URL: https://github.com/apache/incubator-pinot/pull/4222#discussion_r285865713
##########
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 {
Review comment:
Not related to this pr but is it hard to rename this to `ServerStarter`?
(also change `HelixBrokerStarter` to `BrokerStarter`) - Since you changed
`helixServerConfig` to `serverConf`:)
It would be good to align the name of `XXStarter`.
----------------------------------------------------------------
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
