[GitHub] [incubator-druid] pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays
pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays URL: https://github.com/apache/incubator-druid/pull/8841#discussion_r347755811 ## File path: docs/operations/api-reference.md ## @@ -752,7 +752,11 @@ Returns segment information lists including server locations for the given datas * `/druid/broker/v1/loadstatus` -Returns a flag indicating if the Broker knows about all segments in Zookeeper. This can be used to know when a Broker process is ready to be queried after a restart. +Returns a flag indicating if the Broker knows about all segments in the cluster. This can be used to know when a Broker process is ready to be queried after a restart. + +* `/druid/broker/v1/readiness` Review comment: @fjy I am not very concerned about what should be the name of the endpoint, my opinion is that we need an endpoint that returns meaningful HTTP status code so that integration with monitoring/orchestration solutions becomes easier. So I think either we can expose just `/readiness` for all node types that have initialization delay or just add `/druid/broker/v1/readiness` to broker as `/druid/historical/v1/readiness` already exists for historicals. What do you think ? 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays
pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays URL: https://github.com/apache/incubator-druid/pull/8841#discussion_r345325843 ## File path: docs/operations/api-reference.md ## @@ -752,7 +752,11 @@ Returns segment information lists including server locations for the given datas * `/druid/broker/v1/loadstatus` -Returns a flag indicating if the Broker knows about all segments in Zookeeper. This can be used to know when a Broker process is ready to be queried after a restart. +Returns a flag indicating if the Broker knows about all segments in the cluster. This can be used to know when a Broker process is ready to be queried after a restart. + +* `/druid/broker/v1/readiness` Review comment: @nishantmonu51 Historical already has `/druid/historical/v1/readiness` so just extended it for broker - `/druid/broker/v1/readiness` to be consistent, but can change it to just `/readiness` if required, no preference from my side. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays
pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays URL: https://github.com/apache/incubator-druid/pull/8841#discussion_r345325843 ## File path: docs/operations/api-reference.md ## @@ -752,7 +752,11 @@ Returns segment information lists including server locations for the given datas * `/druid/broker/v1/loadstatus` -Returns a flag indicating if the Broker knows about all segments in Zookeeper. This can be used to know when a Broker process is ready to be queried after a restart. +Returns a flag indicating if the Broker knows about all segments in the cluster. This can be used to know when a Broker process is ready to be queried after a restart. + +* `/druid/broker/v1/readiness` Review comment: Historical already has `/druid/historical/v1/readiness` so just extended it for broker - `/druid/broker/v1/readiness` to be consistent, but can change it to just `/readiness` if required, no preference from my side. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [incubator-druid] pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays
pjain1 commented on a change in pull request #8841: add readiness endpoints to processes having initialization delays URL: https://github.com/apache/incubator-druid/pull/8841#discussion_r344310873 ## File path: docs/operations/api-reference.md ## @@ -752,7 +752,11 @@ Returns segment information lists including server locations for the given datas * `/druid/broker/v1/loadstatus` -Returns a flag indicating if the Broker knows about all segments in Zookeeper. This can be used to know when a Broker process is ready to be queried after a restart. +Returns a flag indicating if the Broker knows about all segments in the cluster. This can be used to know when a Broker process is ready to be queried after a restart. + +* `/druid/broker/v1/readiness` Review comment: `/readiness` already exists for historicals so I just extended it for brokers as well, the main reason for its existence is to know when the node is ready to serve queries as opposed to `/status` which just tells if process is up or not. Because of this reason I think we need two endpoints - one for checking when the process is ready to serve and another for checking if the process if up or not. Many monitoring systems rely on HTTP response codes to make decisions about service availability/readiness that's why I think `/readiness` was added even though `/loadstatus` already existed. BTW for health also we have two endpoints `/status` and `/status/health` introduced in https://github.com/apache/incubator-druid/pull/5087. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
