feat(bench): make gunicorn keep-alive and worker-tmp-dir configurable#432
feat(bench): make gunicorn keep-alive and worker-tmp-dir configurable#432mrrobot47 wants to merge 2 commits into
Conversation
The gunicorn --keep-alive and --worker-tmp-dir flags are not supported in the generated web-server wrapper. Because the wrapper is regenerated on setup/restart/migrate, operators who add these flags by hand lose the customization on the next FM operation. Expose them via common_site_config.json (gunicorn_keep_alive, gunicorn_worker_tmp_dir), consistent with the existing gunicorn_workers / gunicorn_max_requests / gunicorn_threads keys. Defaults preserve current behavior exactly: both flags are omitted from the command unless configured, so the generated command is byte-identical to today's output when the keys are unset. gunicorn_keep_alive is emitted whenever it is set (including 0), while gunicorn_worker_tmp_dir is emitted when it is a non-empty path.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #432 +/- ##
==========================================
Coverage ? 41.09%
==========================================
Files ? 126
Lines ? 11846
Branches ? 0
==========================================
Hits ? 4868
Misses ? 6978
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds persistent configuration support for two additional Gunicorn runtime flags (--keep-alive and --worker-tmp-dir) by sourcing new keys from common_site_config.json and emitting them into the generated fm-web-server.sh wrapper when set, with documentation updates to match.
Changes:
- Read
gunicorn_keep_alive/gunicorn_worker_tmp_dirfromcommon_site_config.jsonand pass them through the supervisor render context. - Conditionally append
--keep-aliveand--worker-tmp-dirto the generated Gunicorn command line. - Document the new optional keys in
docs/reference/workers.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frappe_manager/site_manager/modules/bench_supervisor.py | Adds two new config-derived Gunicorn flags to the generated wrapper command line. |
| docs/reference/workers.md | Documents the new optional Gunicorn flags and how to configure them. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ments
gunicorn_worker_tmp_dir is a free-form string from common_site_config.json and
was interpolated raw into the GUNICORN_ARGS="..." wrapper, so a value containing
shell-special characters ($, backtick, quote, backslash, ;) could break the
wrapper or expand at runtime. Wrap both optional-flag values with shlex.quote so
they are always emitted as a single safe shell token.
This does not change output for normal values: shlex.quote("5") == "5" and
shlex.quote("/dev/shm") == "/dev/shm", and with both keys unset the generated
command remains byte-identical to before.
Also correct the two inline comments to match the actual emission rules:
gunicorn_keep_alive is omitted only when unset (0 is still emitted via the
is-not-None guard), and gunicorn_worker_tmp_dir is omitted when unset or empty
(truthiness guard).
Problem
frappe_manager/site_manager/modules/bench_supervisor.pybuilds the gunicorn command line for the generated web-server wrapper in_write_gunicorn_wrapper. Two useful gunicorn runtime flags are not supported there:--keep-alive— how long gunicorn keeps an idle client connection open between requests.--worker-tmp-dir— where gunicorn places its per-worker heartbeat files (commonly pointed at atmpfssuch as/dev/shm).Because the wrapper is regenerated on
setup/restart/migrate, an operator who edits the generated script by hand to add either flag loses that customization on the very next FM operation. There is currently no supported way to set these values persistently.Solution
Expose both flags via
common_site_config.json, consistent with the existinggunicorn_workers/gunicorn_max_requests/gunicorn_threadskeys:gunicorn_keep_alive--keep-alivegunicorn_worker_tmp_dir--worker-tmp-dirgenerate_supervisor_configreads the two keys and adds them to the render context;_write_gunicorn_wrapperconditionally appends the flags only when they are configured.Backward compatibility
Defaults preserve current behavior exactly. When neither key is set, the generated command is byte-identical to today's output — no
--keep-alive, no--worker-tmp-dir.Default (both keys unset):
With
gunicorn_keep_alive=5andgunicorn_worker_tmp_dir=/dev/shm:Emission rules:
gunicorn_keep_aliveuses anis not Nonecheck, so an explicit0still emits--keep-alive 0(a valid gunicorn value); leaving the key unset omits the flag.gunicorn_worker_tmp_diris emitted only when set to a non-empty path.Usage
Scope notes
--graceful-timeoutis intentionally not touched here — it is left hardcoded at30, exactly as ondevelop, because making it configurable is already handled by fix(gunicorn): align --graceful-timeout with --timeout and expose as config #418. This PR is scoped to the two flags that no open PR covers.--keep-alive(idle client-connection lifetime on the gunicorn side) complements the nginx-to-gunicorn upstream keepalive added in perf(nginx): enable HTTP/1.1 keepalive between nginx and gunicorn upstream #417; the two operate at different layers and are independent.Docs: adds an "Optional Gunicorn Flags" subsection to
docs/reference/workers.mddocumenting the two new keys.