xiangfu0 commented on issue #7864:
URL: https://github.com/apache/pinot/issues/7864#issuecomment-989221113
> > For shutdown phase, both `PinotServiceManager` and Broker healthcheck
should fail when you send the signal.
>
> This is not the case as `PinotServiceManager` tries to stop the broker
first, and the broker sleeps for default 10 seconds then just deregisters the
handler. But #7880 seems to do all right here by just failing the
PinotServiceManager instead. Are we comfortable rolling this out as the new
default for everyone?
>
> > This PR enables health check when all bootstrap services are ready. When
sending the signal, PinotServiceManager disable healthcheck first.
>
> Thank you for doing this. This was my other thought at how to approach it,
and it does cover all the services at once. I just have concerns that this
approach is extremely prone to random deadlocks.
>
> ```
> public static Status getServiceStatus() {
> return getServiceStatus(SERVICE_STATUS_CALLBACK);
> }
> ```
>
> really needs to be limited in usage to only the `/health` endpoint. That
said, I guess if you make it deadlock, then the integration tests fail, so
maybe this is ok.
True, I'm making changes to not add PinotSM status check when the SM port <
0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
