-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Health endpoint verifies Druid brokers' health and readiness #99
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor findings but looks good at overall.
docs/health-checking.md
Outdated
|
||
It can be checked by sending a GET request to a `healthEndpoint` path defined in Turnilo's server [configuration](configuration.md). | ||
|
||
Healthy Turnilo instance responds with HTTP status 200 while an unhealthy one responds with the status of 500. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would return 503 as a more specific error.
src/common/models/cluster/cluster.ts
Outdated
@@ -82,6 +84,7 @@ function ensureNotTiny(v: number): void { | |||
export class Cluster extends BaseImmutable<ClusterValue, ClusterJS> { | |||
static TYPE_VALUES: SupportedType[] = ['druid', 'mysql', 'postgres']; | |||
static DEFAULT_TIMEOUT = 40000; | |||
static DEFAULT_HEALTH_CHECKING_TIMEOUT = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no continuous form in other configuration properties. So I would use default_health_check_timeout.
src/server/routes/health/health.ts
Outdated
const unhealthyHttpStatus = 500; | ||
const healthyHttpStatus = 200; | ||
|
||
type ClusterHealthStatus = typeof healthyStatus | typeof unhealthyStatus; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider enum
Fixes #82