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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
