Skip to content

feat(admin): Cap admin HTTP requests with a web-layer timeout#8169

Draft
pbhandari wants to merge 1 commit into
masterfrom
claude/add-granian-timeout
Draft

feat(admin): Cap admin HTTP requests with a web-layer timeout#8169
pbhandari wants to merge 1 commit into
masterfrom
claude/add-granian-timeout

Conversation

@pbhandari

@pbhandari pbhandari commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What

Adds a hard cap on how long a single snuba-admin HTTP request may run at the web layer. Admin is served by Granian (WSGI), which — unlike gunicorn — exposes no per-request timeout, so this is enforced inside the admin Flask app itself and is therefore scoped to admin only.

How

RequestTimeoutMiddleware (snuba/admin/request_timeout.py) wraps application.wsgi_app. It runs each request in a daemon worker thread and joins with a deadline:

  • On overrun it returns 408 Request Timeout with the same JSON error shape the other admin handlers use.
  • Otherwise it passes the inner response (status/headers/body) through untouched, so existing error handlers — including the global exception handler — keep working.

Timeout is configurable via ADMIN_REQUEST_TIMEOUT_SECONDS (env var, default 30s), set just above ClickHouse's existing 25s query cap so CH's own timeout fires first in the normal case and this stays a backstop.

Known limitation

WSGI can't preempt a running thread, so on timeout the client gets its 408 immediately but the abandoned worker thread runs to completion. This is acceptable here because ClickHouse queries are themselves capped at 25s, so the thread unwinds on its own shortly after. The middleware abandons the request; it does not kill the underlying work.

Potential alternative: pebble

If we ever want true termination (actually killing in-flight work rather than abandoning it), pebble is the actively-maintained option — it runs work in a separate process that can be terminated on timeout. The tradeoff is that every request would have to pickle its args/results across a process boundary, which isn't worth it for a WSGI hot path given ClickHouse already self-caps. Noting it here for future reference.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant