fix(ops): make /health a readiness probe, and bound how long the pool waits
`/health` returned the literal string "ok" and touched nothing. Every request in this app needs the database, so that answered a question nobody asked: the container reported healthy while every request 500'd, and with no operator watching during the event there was no signal at all. It now runs `SELECT 1` with a 2s timeout. Verified live against the production stack: 200, stop Postgres, 503 "database timeout", start Postgres, 200 again — with no app restart, because sqlx revalidates on acquire. Deliberately NOT wired to automatic recovery. Compose's `restart: unless-stopped` does not react to healthcheck state anyway, and an autoheal sidecar would be actively wrong here: it would truncate every in-flight upload to "fix" an outage that, as the test above shows, clears on its own. This is a diagnostic — including for the runbook's event-day `curl`. The pool had only `max_connections` set. Three additions: - `acquire_timeout(5s)`. sqlx defaults to 30, so a DB blip parked every request AND all ~100 SSE session revalidations for half a minute before erroring — the app looked hung rather than degraded, and the backlog outlived the blip. - `min_connections(2)`, so the first request after the setup-to-guests-arriving gap doesn't pay TCP + auth. - `statement_timeout=15s` / `lock_timeout=5s` per connection. Without them a single pathological query holds a pool slot indefinitely and no client-side timeout can take it back, because the slot is only released when Postgres finishes. Those two SETs are sent as two statements. `sqlx::query` uses the extended query protocol, which permits exactly one per call — as `SET a; SET b` every new connection failed, which surfaced as the pool never opening one and `create_pool` reporting a connect timeout. Caught by booting against an empty database rather than a warm one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -54,6 +54,31 @@ pub async fn create_pool(database_url: &str) -> Result<PgPool> {
|
||||
|
||||
let pool = match PgPoolOptions::new()
|
||||
.max_connections(max_connections)
|
||||
// Fail fast instead of parking. sqlx's default is 30s, which on a DB blip means every
|
||||
// request AND all ~100 SSE session revalidations sit on the pool for half a minute
|
||||
// before erroring — the app looks hung rather than degraded, and the backlog outlives
|
||||
// the blip. Five seconds is far longer than a healthy acquire ever takes.
|
||||
.acquire_timeout(std::time::Duration::from_secs(5))
|
||||
// Keep a couple of connections warm so the first request after an idle stretch (the gap
|
||||
// between setting the venue up and the guests arriving) doesn't pay TCP + auth.
|
||||
.min_connections(2)
|
||||
// Bound every statement server-side. Without this a single pathological query holds a
|
||||
// pool slot indefinitely and no client-side timeout can take it back — the slot is only
|
||||
// released when Postgres finishes. `lock_timeout` covers the same hazard for a row lock
|
||||
// contended by, say, a release running against an in-flight upload.
|
||||
.after_connect(|conn, _meta| {
|
||||
Box::pin(async move {
|
||||
// Two statements, two round-trips, deliberately. `sqlx::query` uses the extended
|
||||
// query protocol, which permits exactly ONE statement per call — sending them as
|
||||
// `SET a; SET b` makes every new connection fail, which surfaces as the pool
|
||||
// never opening one at all and `create_pool` reporting a connect timeout.
|
||||
sqlx::query("SET statement_timeout = '15s'")
|
||||
.execute(&mut *conn)
|
||||
.await?;
|
||||
sqlx::query("SET lock_timeout = '5s'").execute(conn).await?;
|
||||
Ok(())
|
||||
})
|
||||
})
|
||||
.connect(database_url)
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -129,6 +129,7 @@ async fn main() -> Result<()> {
|
||||
.route("/api/v1/feed", get(handlers::feed::feed))
|
||||
.route("/api/v1/feed/delta", get(handlers::feed::feed_delta))
|
||||
.route("/api/v1/hashtags", get(handlers::feed::hashtags))
|
||||
.route("/api/v1/uploaders", get(handlers::feed::uploaders))
|
||||
// Social
|
||||
.route(
|
||||
"/api/v1/upload/{id}/like",
|
||||
@@ -248,7 +249,7 @@ async fn main() -> Result<()> {
|
||||
// four subtrees. Deleting the route removes the vector outright rather than racing the
|
||||
// decoder; `/media/**` now 404s regardless of encoding.
|
||||
let router = Router::new()
|
||||
.route("/health", get(|| async { "ok" }))
|
||||
.route("/health", get(health))
|
||||
.merge(api)
|
||||
.layer(TraceLayer::new_for_http())
|
||||
.with_state(state);
|
||||
@@ -268,6 +269,47 @@ async fn main() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// How long `/health` waits for the database before calling the app unhealthy. Deliberately
|
||||
/// short: the point is to answer "can this process actually serve a request right now", and a
|
||||
/// probe that blocks for the acquire timeout is itself a symptom.
|
||||
const HEALTH_DB_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(2);
|
||||
|
||||
/// Readiness probe — the Docker healthcheck, Caddy's `depends_on` gate, and the runbook's
|
||||
/// event-day `curl` all hit this.
|
||||
///
|
||||
/// It used to return the literal string `"ok"` and touch nothing. Every request in the app needs
|
||||
/// the database, so that answered a question nobody asked: the container reported healthy while
|
||||
/// every real request 500'd, and with no operator watching during the event there was no signal
|
||||
/// at all. Note what this does NOT buy: Compose's `restart: unless-stopped` does not react to
|
||||
/// healthcheck state, so nothing restarts on a red probe — this is a diagnostic, and it is
|
||||
/// deliberately not wired to automatic recovery, because the pool already heals itself across a
|
||||
/// Postgres restart (sqlx revalidates on acquire) and an auto-restart would truncate every
|
||||
/// in-flight upload to "fix" an outage that was about to clear on its own.
|
||||
async fn health(
|
||||
axum::extract::State(state): axum::extract::State<AppState>,
|
||||
) -> impl axum::response::IntoResponse {
|
||||
use axum::http::StatusCode;
|
||||
match tokio::time::timeout(
|
||||
HEALTH_DB_TIMEOUT,
|
||||
sqlx::query("SELECT 1").execute(&state.pool),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(Ok(_)) => (StatusCode::OK, "ok"),
|
||||
Ok(Err(e)) => {
|
||||
tracing::error!(error = ?e, "health check: database query failed");
|
||||
(StatusCode::SERVICE_UNAVAILABLE, "database unavailable")
|
||||
}
|
||||
Err(_) => {
|
||||
tracing::error!(
|
||||
timeout_s = HEALTH_DB_TIMEOUT.as_secs(),
|
||||
"health check: database did not respond"
|
||||
);
|
||||
(StatusCode::SERVICE_UNAVAILABLE, "database timeout")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Hard cap on how long we wait for in-flight connections to drain after a shutdown
|
||||
/// signal. Uploads (streamed to disk) finish in well under this; the cap exists because
|
||||
/// long-lived SSE streams never end on their own and would otherwise keep the graceful
|
||||
|
||||
Reference in New Issue
Block a user