[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6039: WIP: ServiceManager ADD_TABLE role

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

mcvsubbu commented on a change in pull request #6039:
URL: https://github.com/apache/incubator-pinot/pull/6039#discussion_r499763424



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##########
@@ -350,6 +350,8 @@ private void setUpPinotController() {
     LOGGER.info("Starting Pinot Helix resource manager and connecting to Zookeeper");
     _helixResourceManager.start(_helixParticipantManager);
 
+    // TODO: ideally we can block to do schema/table install here, as it is before the thing is listening..

Review comment:
       Why would you want to block before schema/table install? A controller can start up without any tables just fine

##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManager.java
##########
@@ -92,6 +92,8 @@ public String startRole(ServiceRole role, Map<String, Object> properties)
         return startBroker(new PinotConfiguration(properties));
       case SERVER:
         return startServer(new PinotConfiguration(properties));
+      case ADD_TABLE:

Review comment:
       This is not a pinot service. Perhaps you should pull out a base class, and add a `PinotSetupServiceManager` (I am sure there is a better name) in which you can add tables, ingest data, check for data being online, etc.? 

##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/services/ServiceRole.java
##########
@@ -22,5 +22,5 @@
  * ServiceRole defines a role that Pinot Service could start/stop.
  */
 public enum ServiceRole {
-  CONTROLLER, BROKER, SERVER, MINION,
+  CONTROLLER, BROKER, SERVER, MINION, ADD_TABLE

Review comment:
       `ADD_TABLE` is not a service in pinot, it should be an external service.




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