You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/09/07 19:10:26 UTC

[GitHub] [pinot] ckharide opened a new issue #7404: startServiceManager() not used in StartServiceManagerCommand

ckharide opened a new issue #7404:
URL: https://github.com/apache/pinot/issues/7404


   Hello -  I just started looking at the code and please ignore if this issue is addressed . 
   
   It seems the startServiceManager() method in StartServiceManagerCommand is not used also am curious why it returns the instanceID when the pinotServiceManager thread wouldn't have started .  Do you think  it would be good to return the instance ID only once the pinotServiceManager started successfully by using the state of the isStarted() method.  
   
   Let me know what do you think and i would be intersted to work on it if you permit .
   
   


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


[GitHub] [pinot] Jackie-Jiang commented on issue #7404: startServiceManager() not used in StartServiceManagerCommand

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on issue #7404:
URL: https://github.com/apache/pinot/issues/7404#issuecomment-916492966


   No it is not mutated, but fixed within the constructor (the port is already picked within the constructor). Alternatively we can also pick the port within the `start()` and fix the instanceId there, but in most cases the constructor and `start()` are invoked together so no big difference there.
   Adding @xiangfu0 to this thread


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


[GitHub] [pinot] Jackie-Jiang commented on issue #7404: startServiceManager() not used in StartServiceManagerCommand

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on issue #7404:
URL: https://github.com/apache/pinot/issues/7404#issuecomment-914643229


   It is called in the `execute()` method. Did you run into issues with `StartServiceManagerCommand`?


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


[GitHub] [pinot] ckharide commented on issue #7404: startServiceManager() not used in StartServiceManagerCommand

Posted by GitBox <gi...@apache.org>.
ckharide commented on issue #7404:
URL: https://github.com/apache/pinot/issues/7404#issuecomment-914927759


   Ah thank you . No i didnt run into any issue but wondering whats the point in returning instanceID . 


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


[GitHub] [pinot] ckharide commented on issue #7404: startServiceManager() not used in StartServiceManagerCommand

Posted by GitBox <gi...@apache.org>.
ckharide commented on issue #7404:
URL: https://github.com/apache/pinot/issues/7404#issuecomment-915763649


   Ok may be am missing something here - in PinotServiceManager constructor _instanceId is already there. 
   _instanceId = String.format("ServiceManager_%s_%d", hostname, port);
   
   Does it get mutated after the StartServiceManagerCommand.startServiceManager()


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


[GitHub] [pinot] ckharide edited a comment on issue #7404: startServiceManager() not used in StartServiceManagerCommand

Posted by GitBox <gi...@apache.org>.
ckharide edited a comment on issue #7404:
URL: https://github.com/apache/pinot/issues/7404#issuecomment-914927759


   Ah thank you . No i didnt run into any issue but wondering whats the thought in returning instanceID , prior to service getting started . 


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


[GitHub] [pinot] Jackie-Jiang commented on issue #7404: startServiceManager() not used in StartServiceManagerCommand

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on issue #7404:
URL: https://github.com/apache/pinot/issues/7404#issuecomment-915664185


   Actually the instanceId is returned after it is started (`StartServiceManagerCommand.startServiceManager()`), and it is logged in `StartServiceManagerCommand.startPinotService()`


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

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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