Merge pull request frappe#1387 from ankush/max_requests

feat: add `max_requests` to gunicorn args
Zawala · Nov 23, 2022 · d198465 · d198465
2 parents 965e178 + c4305fd
commit d198465
Showing 10 changed files with 59 additions and 606 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -29,8 +29,8 @@ repos:
       - id: black
         additional_dependencies: ['click==8.0.4']
 
-  - repo: https://gitlab.com/pycqa/flake8
-    rev: 3.9.2
+  - repo: https://github.com/pycqa/flake8
+    rev: 5.0.4
     hooks:
       - id: flake8
         additional_dependencies: ['flake8-bugbear',]

diff --git a/bench/config/common_site_config.py b/bench/config/common_site_config.py
@@ -15,6 +15,8 @@
 	"live_reload": True,
 }
 
+DEFAULT_MAX_REQUESTS = 5000
+
 
 def setup_config(bench_path):
 	make_pid_folder(bench_path)
@@ -62,6 +64,20 @@ def get_gunicorn_workers():
 	return {"gunicorn_workers": multiprocessing.cpu_count() * 2 + 1}
 
 
+def compute_max_requests_jitter(max_requests: int) -> int:
+	return int(max_requests * 0.1)
+
+
+def get_default_max_requests(worker_count: int):
+	"""Get max requests and jitter config based on number of available workers."""
+
+	if worker_count <= 1:
+		# If there's only one worker then random restart can cause spikes in response times and
+		# can be annoying. Hence not enabled by default.
+		return 0
+	return DEFAULT_MAX_REQUESTS
+
+
 def update_config_for_frappe(config, bench_path):
 	ports = make_ports(bench_path)
 

diff --git a/bench/config/supervisor.py b/bench/config/supervisor.py
@@ -8,7 +8,12 @@
 from bench.app import use_rq
 from bench.utils import get_bench_name, which
 from bench.bench import Bench
-from bench.config.common_site_config import update_config, get_gunicorn_workers
+from bench.config.common_site_config import (
+	update_config,
+	get_gunicorn_workers,
+	get_default_max_requests,
+	compute_max_requests_jitter,
+)
 
 # imports - third party imports
 import click
@@ -26,6 +31,13 @@ def generate_supervisor_config(bench_path, user=None, yes=False, skip_redis=Fals
 	template = bench.config.env().get_template("supervisor.conf")
 	bench_dir = os.path.abspath(bench_path)
 
+	web_worker_count = config.get(
+		"gunicorn_workers", get_gunicorn_workers()["gunicorn_workers"]
+	)
+	max_requests = config.get(
+		"gunicorn_max_requests", get_default_max_requests(web_worker_count)
+	)
+
 	config = template.render(
 		**{
 			"bench_dir": bench_dir,
@@ -39,9 +51,9 @@ def generate_supervisor_config(bench_path, user=None, yes=False, skip_redis=Fals
 			"redis_socketio_config": os.path.join(bench_dir, "config", "redis_socketio.conf"),
 			"redis_queue_config": os.path.join(bench_dir, "config", "redis_queue.conf"),
 			"webserver_port": config.get("webserver_port", 8000),
-			"gunicorn_workers": config.get(
-				"gunicorn_workers", get_gunicorn_workers()["gunicorn_workers"]
-			),
+			"gunicorn_workers": web_worker_count,
+			"gunicorn_max_requests": max_requests,
+			"gunicorn_max_requests_jitter": compute_max_requests_jitter(max_requests),
 			"bench_name": get_bench_name(bench_path),
 			"background_workers": config.get("background_workers") or 1,
 			"bench_cmd": which("bench"),

diff --git a/bench/config/systemd.py b/bench/config/systemd.py
@@ -9,7 +9,12 @@
 import bench
 from bench.app import use_rq
 from bench.bench import Bench
-from bench.config.common_site_config import get_gunicorn_workers, update_config
+from bench.config.common_site_config import (
+	get_gunicorn_workers,
+	update_config,
+	get_default_max_requests,
+	compute_max_requests_jitter,
+)
 from bench.utils import exec_cmd, which, get_bench_name
 
 
@@ -61,6 +66,13 @@ def generate_systemd_config(
 			get_bench_name(bench_path) + "-frappe-long-worker@" + str(i + 1) + ".service"
 		)
 
+	web_worker_count = config.get(
+		"gunicorn_workers", get_gunicorn_workers()["gunicorn_workers"]
+	)
+	max_requests = config.get(
+		"gunicorn_max_requests", get_default_max_requests(web_worker_count)
+	)
+
 	bench_info = {
 		"bench_dir": bench_dir,
 		"sites_dir": os.path.join(bench_dir, "sites"),
@@ -73,9 +85,9 @@ def generate_systemd_config(
 		"redis_socketio_config": os.path.join(bench_dir, "config", "redis_socketio.conf"),
 		"redis_queue_config": os.path.join(bench_dir, "config", "redis_queue.conf"),
 		"webserver_port": config.get("webserver_port", 8000),
-		"gunicorn_workers": config.get(
-			"gunicorn_workers", get_gunicorn_workers()["gunicorn_workers"]
-		),
+		"gunicorn_workers": web_worker_count,
+		"gunicorn_max_requests": max_requests,
+		"gunicorn_max_requests_jitter": compute_max_requests_jitter(max_requests),
 		"bench_name": get_bench_name(bench_path),
 		"worker_target_wants": " ".join(background_workers),
 		"bench_cmd": which("bench"),

diff --git a/bench/config/templates/supervisor.conf b/bench/config/templates/supervisor.conf
@@ -3,7 +3,7 @@
 ; killasgroup=true --> send kill signal to child processes too
 
 [program:{{ bench_name }}-frappe-web]
-command={{ bench_dir }}/env/bin/gunicorn -b 127.0.0.1:{{ webserver_port }} -w {{ gunicorn_workers }} -t {{ http_timeout }} frappe.app:application --preload
+command={{ bench_dir }}/env/bin/gunicorn -b 127.0.0.1:{{ webserver_port }} -w {{ gunicorn_workers }} --max-requests {{ gunicorn_max_requests }} --max-requests-jitter {{ gunicorn_max_requests_jitter }} -t {{ http_timeout }} frappe.app:application --preload
 priority=4
 autostart=true
 autorestart=true

diff --git a/bench/config/templates/systemd/frappe-bench-frappe-web.service b/bench/config/templates/systemd/frappe-bench-frappe-web.service
@@ -6,7 +6,7 @@ PartOf={{ bench_name }}-web.target
 User={{ user }}
 Group={{ user }}
 Restart=always
-ExecStart={{ bench_dir }}/env/bin/gunicorn -b 127.0.0.1:{{ webserver_port }} -w {{ gunicorn_workers }} -t {{ http_timeout }} frappe.app:application --preload
+ExecStart={{ bench_dir }}/env/bin/gunicorn -b 127.0.0.1:{{ webserver_port }} -w {{ gunicorn_workers }} -t {{ http_timeout }} --max-requests {{ gunicorn_max_requests }} --max-requests-jitter {{ gunicorn_max_requests_jitter }} frappe.app:application --preload
 StandardOutput=file:{{ bench_dir }}/logs/web.log
 StandardError=file:{{ bench_dir }}/logs/web.error.log
 WorkingDirectory={{ sites_dir }}
diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py
@@ -125,7 +125,10 @@ def check_latest_version():
 		local_version = Version(VERSION)
 
 		if pypi_version > local_version:
-			log(f"A newer version of bench is available: {local_version} → {pypi_version}", stderr=True)
+			log(
+				f"A newer version of bench is available: {local_version} → {pypi_version}",
+				stderr=True,
+			)
 
 
 def pause_exec(seconds=10):

diff --git a/bench/utils/app.py b/bench/utils/app.py
@@ -185,7 +185,9 @@ def get_required_deps(org, name, branch, deps="hooks.py"):
 	res = requests.get(url=git_api_url, params=params).json()
 
 	if "message" in res:
-		git_url = f"https://raw.githubusercontent.com/{org}/{name}/{params['ref']}/{name}/{deps}"
+		git_url = (
+			f"https://raw.githubusercontent.com/{org}/{name}/{params['ref']}/{name}/{deps}"
+		)
 		return requests.get(git_url).text
 
 	return base64.decodebytes(res["content"].encode()).decode()

diff --git a/docs/easy_install.md b/docs/easy_install.md