fix(gunicorn): align --graceful-timeout with --timeout and expose as config

[mrrobot47]

Issue

frappe_manager/site_manager/modules/bench_supervisor.py (lines 381-391) hardcodes --graceful-timeout 30 on the gunicorn command line while -t (--timeout) is sourced from context['http_timeout'] (default 120, configurable via common_site_config.json):

gunicorn_args = (
    f"-b 0.0.0.0:{context['webserver_port']}"
    f" -w {context['gunicorn_workers']}"
    ...
    f" -t {context['http_timeout']}"
    f" --graceful-timeout 30"
    f" frappe.app:application --preload"
)

When a worker hits --max-requests (or any other SIGTERM), gunicorn gives in-flight requests only 30 seconds to finish before SIGKILL fires — even though those same requests are otherwise allowed up to http_timeout (default 120s) to run. The SIGKILL closes upstream sockets abruptly, which surfaces at the nginx layer as upstream RSTs and ultimately as 502s returned to clients.

Impact

Affects every FM-deployed site running under sustained load, where --max-requests recycles happen frequently enough to coincide with in-flight long-running requests. The failure mode is non-deterministic 502s during otherwise-normal worker recycling: a request that would have completed within the configured http_timeout gets SIGKILL'd mid-flight and the client sees a 502 instead of a successful response.

APM data on FM-deployed production-scale installs measured worker-recycle RST events as a meaningful contributor to chronic 5xx rate. The 30s graceful window being shorter than the 120s request timeout is the root cause: any request that takes 30-120s — slow custom-app endpoints, large reports, sync HTTP calls to external services, large frappe.db operations — is at risk during a routine recycle.

This is a configuration-only issue; the upstream gunicorn default of --graceful-timeout 30 is not appropriate when --timeout is set to a value larger than 30.

Solution

Expose --graceful-timeout as a common_site_config.json knob (gunicorn_graceful_timeout), defaulting to http_timeout for backward-compatible behavior:

• New field in generate_supervisor_config context: gunicorn_graceful_timeout = config.get("gunicorn_graceful_timeout", http_timeout).
• _write_gunicorn_wrapper now emits --graceful-timeout {context['gunicorn_graceful_timeout']} instead of the hardcoded 30.

The default is http_timeout (not the previous 30) on the principle that the graceful drain window should match the request timeout: any request gunicorn was willing to let run synchronously should also be allowed to drain on shutdown. Operators who specifically need a shorter drain (e.g. to bound rolling-restart window length) can set gunicorn_graceful_timeout lower than http_timeout in common_site_config.json.

Alternatives considered:

• Minimum-viable: just change the literal 30 to {context['http_timeout']} without adding a new knob. Rejected because some operators legitimately want a shorter graceful window to bound restart duration — exposing the knob costs ~3 lines and preserves that flexibility.

Test plan

1. Spin up an fm create site with the default common_site_config.json. Inspect config/fm-web-server.sh — --graceful-timeout 120 (matches default http_timeout).
2. Set "gunicorn_graceful_timeout": 60 in common_site_config.json, re-run fm bench supervisor setup --force. Inspect the wrapper — --graceful-timeout 60.
3. Set "http_timeout": 180 (no gunicorn_graceful_timeout), re-run setup. Inspect the wrapper — --graceful-timeout 180.
4. Behavior: under load, induce a worker recycle (set gunicorn_max_requests low, send sustained traffic). Confirm no 502 spikes from RST during recycles for requests that complete within http_timeout.

Rollback

Revert the merge commit. The config field is opt-in (read with config.get(..., http_timeout)), so on rollback all existing sites continue to work with the previously-hardcoded 30.

References

• frappe_manager/site_manager/modules/bench_supervisor.py:381-391 — _write_gunicorn_wrapper constructing gunicorn_args
• frappe_manager/site_manager/modules/bench_supervisor.py:218-236 — generate_supervisor_config context dict where the new field is sourced
• gunicorn docs: --graceful-timeout semantics — workers receive SIGTERM, then SIGKILL after this many seconds; default 30

[Copilot]

Pull request overview

Aligns Gunicorn’s shutdown drain behavior with the configured HTTP request timeout by replacing a hardcoded --graceful-timeout 30 with a configurable value sourced from common_site_config.json, defaulting to http_timeout. This aims to reduce 502s caused by workers being SIGKILL’d mid-request during routine recycling.

Changes:

• Introduces gunicorn_graceful_timeout in the supervisor config rendering context, defaulting to http_timeout.
• Updates the generated Gunicorn wrapper args to emit --graceful-timeout {gunicorn_graceful_timeout} instead of a fixed 30.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mrrobot47]

Pushed 1124823. Addressed:

• frappe_manager/templates/supervisor.conf.tmpl:13 — Copilot: hardcoded stopwaitsecs=40 would let supervisor SIGKILL the gunicorn master before a (now larger) graceful drain window completes. Now stopwaitsecs={{ supervisor_web_stopwaitsecs }} where supervisor_web_stopwaitsecs = gunicorn_graceful_timeout + 10 is computed alongside the timeout in bench_supervisor.py:240. Preserves the invariant the template comment on line 5 already documents.
• frappe_manager/site_manager/modules/bench_supervisor.py:218 — Copilot: no type/validity check on JSON-sourced timeouts. Added _coerce_positive_int(value, fallback) and both http_timeout and gunicorn_graceful_timeout go through it; stringified, null, zero, negative, or non-numeric values fall back to the documented default rather than landing in the generated wrapper / supervisor config.

CI: green (Analyze actions / Analyze python / CodeQL / pytest 3.13 all pass).