adriancole commented on a change in pull request #6039: URL: https://github.com/apache/incubator-pinot/pull/6039#discussion_r499930870
########## 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: when a table is in the bootstrap roles for servicemanager, it can make sense that this is a part of bootstrap. Not just controller, but nothing ideally should listen until bootstrap is complete. ServiceManager is effectively a composite service. In talking also at length with @mayankshriv and with your comment also, I can see this point is not understandable outside my head. The impact is more external orchestration, which imho is a bad experience. We are allowing a composite service, but deciding against taking advantage of it. Another point lost despite various attempts to convey it is that I don't mean to literally block here. I mean to add a callback iff servicemanager is in use implements it. otherwise a noop. If this above comment changes things, lemme know. Otherwise, I'm not really sure this PR is worth proceeding with as if we aren't taking these concerns internally, the external approach (shell scripts etc) seems more coherent.. ---------------------------------------------------------------- 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 --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org