[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6134: Adding a flag in PinotServiceManager to allow health check includes all components health check

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

mayankshriv commented on a change in pull request #6134:
URL: https://github.com/apache/incubator-pinot/pull/6134#discussion_r504825339



##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -69,6 +69,8 @@
   private String _clusterName = DEFAULT_CLUSTER_NAME;
   @Option(name = "-port", required = true, metaVar = "<int>", usage = "Pinot service manager admin port, -1 means disable, 0 means a random available port.")
   private int _port;
+  @Option(name = "-healthCheckAllComponents", metaVar = "<boolean>", usage = "Pinot service manager health check returns the all components health check. The health check returns OK when all the components are returning OK.")

Review comment:
       Perhaps we need an enum instead of boolean. For example: healthCheckComponents=CONTROLLER|BROKER? Don't have a concrete use case in mind, so just a soft suggestion at the moment.
   
   Another thought is `stateCheck` instead of `healthCheck`. That we, it can also be used to wait for `bootstrap` of a table.




----------------------------------------------------------------
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



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