Add a http health endpoint #502
Conversation
Also store a reference to the original config inside the context.
src/config/context.rs
Outdated
| // (it will reload its schema before running the query) | ||
|
|
||
| Ok(SeafowlContext { | ||
| config: Arc::new(cfg), |
There was a problem hiding this comment.
Probably don't need the Arc here.
There was a problem hiding this comment.
Yeah, might as well make it an owned struct without the Arc
There was a problem hiding this comment.
I now remembered that I initially put the Arc on top of it due to the scope_to_* methods which create a new context on the heap, but it's probably ok to do it anyway since the config is tiny.
src/frontend/http.rs
Outdated
|
|
||
| // Health-check/readiness probe | ||
| let ctx = context; | ||
| let health_route = warp::path!("readyz") |
There was a problem hiding this comment.
Still not sure whether this is readyz, healthz or both.
There was a problem hiding this comment.
IIUC healthz (liveness) is for when something internal has gone wrong in the system and we need it to be restarted. E.g. we check that some background worker thread is still running and if it's crashed, we say we're unhealthy.
I think we can make healthz and readyz do the same thing for now.
src/config/context.rs
Outdated
| // (it will reload its schema before running the query) | ||
|
|
||
| Ok(SeafowlContext { | ||
| config: Arc::new(cfg), |
There was a problem hiding this comment.
Yeah, might as well make it an owned struct without the Arc
| pub async fn health_endpoint(context: Arc<SeafowlContext>) -> Result<Response, ApiError> { | ||
| #[cfg(feature = "frontend-arrow-flight")] | ||
| if let Some(flight) = &context.config.frontend.flight { | ||
| // TODO: run SELECT 1 or something similar? |
There was a problem hiding this comment.
Depends, does us being able to serve this endpoint mean the Arrow Flight frontend is initialized already? Does the Flight frontend check for connectivity on startup by loading the catalog?
There was a problem hiding this comment.
does us being able to serve this endpoint mean the Arrow Flight frontend is initialized already?
In principle no, since there's a short period during startup when Flight frontend might not have initialized but the HTTP one has. In practice probably yes, since Flight frontend is spawned first, and there is likely a miniscule startup time for both.
Does the Flight frontend check for connectivity on startup by loading the catalog?
No, just create a server listening fore queries. Only when one arrives does it try to load the catalog and resolve the identifiers.
src/frontend/http.rs
Outdated
|
|
||
| // Health-check/readiness probe | ||
| let ctx = context; | ||
| let health_route = warp::path!("readyz") |
There was a problem hiding this comment.
IIUC healthz (liveness) is for when something internal has gone wrong in the system and we need it to be restarted. E.g. we check that some background worker thread is still running and if it's crashed, we say we're unhealthy.
I think we can make healthz and readyz do the same thing for now.
| cached_read_query_route | ||
| .or(uncached_read_write_query_route) | ||
| .or(upload_route) | ||
| .or(health_route) |
There was a problem hiding this comment.
Maybe this should be a separate frontend, since the user can have a gRPC frontend enabled without HTTP. But then gRPC isn't authed and HTTP is, so it doesn't make it less secure.
There was a problem hiding this comment.
Yeah, prefer to leave it there for now, but if there's a strong need can separate it out later on.
Also remove the Arc from the config kept in the context.
In particular this one corresponds to k8s readiness probe. For now it only includes trying to connect to the Flight gRPC endpoint if configured, and if that fails returns 503.