From 77ebdbe6f93e98a46a8926cc2f8a9bac6b83d3f7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 7 Aug 2023 12:27:02 +0530 Subject: [PATCH 1/3] ci: fix test versions (#1474) --- .github/workflows/ci.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5c44260..74b1b0d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,9 +114,17 @@ jobs: - uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} + - uses: actions/setup-node@v3 + if: ${{ matrix.python-version == '3.10' }} + with: + node-version: 18 + + - uses: actions/setup-node@v3 + if: ${{ matrix.python-version == '3.7' }} with: node-version: 14 + - run: | wget https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6-1/wkhtmltox_0.12.6-1.focal_amd64.deb; sudo apt install ./wkhtmltox_0.12.6-1.focal_amd64.deb; From 87efb75a08e6863a3f2e15cec7dbd375b1fa2a7d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 7 Aug 2023 12:55:19 +0530 Subject: [PATCH 2/3] refactor!: Reuse redis_cache instace for socketio (#1382) WARNING: Just a POC. You're free to modify your setup to test this out. Summary: - After this change bench wont generate config for redis_socketio instance. - Instead we set the same port as redis_cache service for use in socketio. i.e. both will share same instance. Code changes: - Config will have this difference: ```diff - "redis_socketio": "redis://localhost:12000", - "redis_cache": "redis://localhost:13000", + "redis_socketio": "redis://localhost:13000", + "redis_cache": "redis://localhost:13000", ``` - supervisord/systemd wont start redis_socketio service - for development setups: redis_socketio service is removed from procfile. Why? - Currently this redis instance is used only to ship messages from python to nodejs server... which is largely stateless (only pub/sub channels) - This change reduces 1 running process. Less resources for this ~= more resources for other running things. TODO: - [ ] decicisions - should we even do this? this should be optinal? - [ ] refactor progressbar in frappe - [ ] Tests - manual, FC, docker, automated - [ ] "migration" plan All dependent deployment projects will likely have to refactor their code to drop this service, shouldn't be too much work. Example: Frappe cloud, Frappe Docker. --- bench/config/common_site_config.py | 2 +- bench/config/redis.py | 10 +--------- bench/config/supervisor.py | 1 - bench/config/systemd.py | 15 --------------- bench/config/templates/Procfile | 1 - bench/config/templates/redis_socketio.conf | 8 -------- bench/config/templates/supervisor.conf | 14 +------------- .../systemd/frappe-bench-redis-socketio.service | 12 ------------ .../templates/systemd/frappe-bench-redis.target | 2 +- bench/tests/test_base.py | 1 - bench/tests/test_init.py | 4 ++-- bench/tests/test_setup_production.py | 2 -- 12 files changed, 6 insertions(+), 66 deletions(-) delete mode 100644 bench/config/templates/redis_socketio.conf delete mode 100644 bench/config/templates/systemd/frappe-bench-redis-socketio.service diff --git a/bench/config/common_site_config.py b/bench/config/common_site_config.py index df58daf1..a3f1f6a2 100644 --- a/bench/config/common_site_config.py +++ b/bench/config/common_site_config.py @@ -100,7 +100,7 @@ def make_ports(bench_path): "socketio_port": 9000, "file_watcher_port": 6787, "redis_queue": 11000, - "redis_socketio": 12000, + "redis_socketio": 13000, "redis_cache": 13000, } diff --git a/bench/config/redis.py b/bench/config/redis.py index 7e4f5af8..bca96991 100644 --- a/bench/config/redis.py +++ b/bench/config/redis.py @@ -15,7 +15,7 @@ def generate_config(bench_path): redis_version = get_redis_version() ports = {} - for key in ("redis_cache", "redis_queue", "redis_socketio"): + for key in ("redis_cache", "redis_queue"): ports[key] = urlparse(config[key]).port write_redis_config( @@ -28,12 +28,6 @@ def generate_config(bench_path): bench_path=bench_path, ) - write_redis_config( - template_name="redis_socketio.conf", - context={"port": ports["redis_socketio"], "redis_version": redis_version}, - bench_path=bench_path, - ) - write_redis_config( template_name="redis_cache.conf", context={ @@ -56,10 +50,8 @@ def generate_config(bench_path): # make ACL files acl_rq_path = os.path.join(bench_path, "config", "redis_queue.acl") acl_redis_cache_path = os.path.join(bench_path, "config", "redis_cache.acl") - acl_redis_socketio_path = os.path.join(bench_path, "config", "redis_socketio.acl") open(acl_rq_path, "a").close() open(acl_redis_cache_path, "a").close() - open(acl_redis_socketio_path, "a").close() def write_redis_config(template_name, context, bench_path): diff --git a/bench/config/supervisor.py b/bench/config/supervisor.py index b78c0470..7b482fa5 100644 --- a/bench/config/supervisor.py +++ b/bench/config/supervisor.py @@ -48,7 +48,6 @@ def generate_supervisor_config(bench_path, user=None, yes=False, skip_redis=Fals "redis_server": which("redis-server"), "node": which("node") or which("nodejs"), "redis_cache_config": os.path.join(bench_dir, "config", "redis_cache.conf"), - "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": web_worker_count, diff --git a/bench/config/systemd.py b/bench/config/systemd.py index a19de662..b4932b5a 100644 --- a/bench/config/systemd.py +++ b/bench/config/systemd.py @@ -82,7 +82,6 @@ def generate_systemd_config( "redis_server": which("redis-server"), "node": which("node") or which("nodejs"), "redis_cache_config": os.path.join(bench_dir, "config", "redis_cache.conf"), - "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": web_worker_count, @@ -244,14 +243,10 @@ def setup_redis_config(bench_info, bench_path): bench_redis_queue_template = bench.config.env().get_template( "systemd/frappe-bench-redis-queue.service" ) - bench_redis_socketio_template = bench.config.env().get_template( - "systemd/frappe-bench-redis-socketio.service" - ) bench_redis_target_config = bench_redis_target_template.render(**bench_info) bench_redis_cache_config = bench_redis_cache_template.render(**bench_info) bench_redis_queue_config = bench_redis_queue_template.render(**bench_info) - bench_redis_socketio_config = bench_redis_socketio_template.render(**bench_info) bench_redis_target_config_path = os.path.join( bench_path, "config", "systemd", bench_info.get("bench_name") + "-redis.target" @@ -262,12 +257,6 @@ def setup_redis_config(bench_info, bench_path): bench_redis_queue_config_path = os.path.join( bench_path, "config", "systemd", bench_info.get("bench_name") + "-redis-queue.service" ) - bench_redis_socketio_config_path = os.path.join( - bench_path, - "config", - "systemd", - bench_info.get("bench_name") + "-redis-socketio.service", - ) with open(bench_redis_target_config_path, "w") as f: f.write(bench_redis_target_config) @@ -278,9 +267,6 @@ def setup_redis_config(bench_info, bench_path): with open(bench_redis_queue_config_path, "w") as f: f.write(bench_redis_queue_config) - with open(bench_redis_socketio_config_path, "w") as f: - f.write(bench_redis_socketio_config) - def _create_symlinks(bench_path): bench_dir = os.path.abspath(bench_path) @@ -319,6 +305,5 @@ def get_unit_files(bench_path): [bench_name + "-node-socketio", ".service"], [bench_name + "-redis-cache", ".service"], [bench_name + "-redis-queue", ".service"], - [bench_name + "-redis-socketio", ".service"], ] return unit_files diff --git a/bench/config/templates/Procfile b/bench/config/templates/Procfile index 899d1290..4ef4b842 100644 --- a/bench/config/templates/Procfile +++ b/bench/config/templates/Procfile @@ -1,6 +1,5 @@ {% if not skip_redis %} redis_cache: redis-server config/redis_cache.conf -redis_socketio: redis-server config/redis_socketio.conf redis_queue: redis-server config/redis_queue.conf {% endif %} web: bench serve {% if webserver_port -%} --port {{ webserver_port }} {%- endif %} diff --git a/bench/config/templates/redis_socketio.conf b/bench/config/templates/redis_socketio.conf deleted file mode 100644 index ff3db75b..00000000 --- a/bench/config/templates/redis_socketio.conf +++ /dev/null @@ -1,8 +0,0 @@ -dbfilename redis_socketio.rdb -dir {{ pid_path }} -pidfile {{ pid_path }}/redis_socketio.pid -bind 127.0.0.1 -port {{ port }} -{% if redis_version and redis_version >= 6.0 %} -aclfile {{ config_path }}/redis_socketio.acl -{% endif %} diff --git a/bench/config/templates/supervisor.conf b/bench/config/templates/supervisor.conf index edf00cf4..240701c9 100644 --- a/bench/config/templates/supervisor.conf +++ b/bench/config/templates/supervisor.conf @@ -152,18 +152,6 @@ user={{ user }} directory={{ sites_dir }} {% endif %} -{% if not skip_redis %} -[program:{{ bench_name }}-redis-socketio] -command={{ redis_server }} {{ redis_socketio_config }} -priority=1 -autostart=true -autorestart=true -stdout_logfile={{ bench_dir }}/logs/redis-socketio.log -stderr_logfile={{ bench_dir }}/logs/redis-socketio.error.log -user={{ user }} -directory={{ sites_dir }} -{% endif %} - {% if node %} [program:{{ bench_name }}-node-socketio] command={{ node }} {{ bench_dir }}/apps/frappe/socketio.js @@ -193,5 +181,5 @@ programs={{ bench_name }}-frappe-workerbeat,{{ bench_name }}-frappe-worker,{{ be {% if not skip_redis %} [group:{{ bench_name }}-redis] -programs={{ bench_name }}-redis-cache,{{ bench_name }}-redis-queue,{{ bench_name }}-redis-socketio +programs={{ bench_name }}-redis-cache,{{ bench_name }}-redis-queue {% endif %} diff --git a/bench/config/templates/systemd/frappe-bench-redis-socketio.service b/bench/config/templates/systemd/frappe-bench-redis-socketio.service deleted file mode 100644 index 3eeafdb9..00000000 --- a/bench/config/templates/systemd/frappe-bench-redis-socketio.service +++ /dev/null @@ -1,12 +0,0 @@ -[Unit] -Description="{{ bench_name }}-redis-socketio" -PartOf={{ bench_name }}-redis.target - -[Service] -User={{ user }} -Group={{ user }} -Restart=always -ExecStart={{ redis_server }} {{ redis_socketio_config }} -StandardOutput=file:{{ bench_dir }}/logs/redis-socketio.log -StandardError=file:{{ bench_dir }}/logs/redis-socketio.error.log -WorkingDirectory={{ sites_dir }} diff --git a/bench/config/templates/systemd/frappe-bench-redis.target b/bench/config/templates/systemd/frappe-bench-redis.target index 5bc14734..88799426 100644 --- a/bench/config/templates/systemd/frappe-bench-redis.target +++ b/bench/config/templates/systemd/frappe-bench-redis.target @@ -1,6 +1,6 @@ [Unit] After=network.target -Wants={{ bench_name }}-redis-cache.service {{ bench_name }}-redis-queue.service {{ bench_name }}-redis-socketio.service +Wants={{ bench_name }}-redis-cache.service {{ bench_name }}-redis-queue.service [Install] WantedBy=multi-user.target diff --git a/bench/tests/test_base.py b/bench/tests/test_base.py index efc1e81f..24ceb4f6 100644 --- a/bench/tests/test_base.py +++ b/bench/tests/test_base.py @@ -67,7 +67,6 @@ class TestBenchBase(unittest.TestCase): def assert_config(self, bench_name): for config, search_key in ( ("redis_queue.conf", "redis_queue.rdb"), - ("redis_socketio.conf", "redis_socketio.rdb"), ("redis_cache.conf", "redis_cache.rdb"), ): diff --git a/bench/tests/test_init.py b/bench/tests/test_init.py index 56634e67..30bb1572 100755 --- a/bench/tests/test_init.py +++ b/bench/tests/test_init.py @@ -55,7 +55,7 @@ class TestBenchInit(TestBenchBase): "socketio_port": 9000, "file_watcher_port": 6787, "redis_queue": "redis://localhost:11000", - "redis_socketio": "redis://localhost:12000", + "redis_socketio": "redis://localhost:13000", "redis_cache": "redis://localhost:13000", }, ) @@ -67,7 +67,7 @@ class TestBenchInit(TestBenchBase): "socketio_port": 9001, "file_watcher_port": 6788, "redis_queue": "redis://localhost:11001", - "redis_socketio": "redis://localhost:12001", + "redis_socketio": "redis://localhost:13001", "redis_cache": "redis://localhost:13001", }, ) diff --git a/bench/tests/test_setup_production.py b/bench/tests/test_setup_production.py index 4dae93c0..06fe3c82 100644 --- a/bench/tests/test_setup_production.py +++ b/bench/tests/test_setup_production.py @@ -103,7 +103,6 @@ class TestSetupProduction(TestBenchBase): f"program:{bench_name}-frappe-web", f"program:{bench_name}-redis-cache", f"program:{bench_name}-redis-queue", - f"program:{bench_name}-redis-socketio", f"group:{bench_name}-web", f"group:{bench_name}-workers", f"group:{bench_name}-redis", @@ -150,7 +149,6 @@ class TestSetupProduction(TestBenchBase): # "{bench_name}-web:{bench_name}-node-socketio[\s]+RUNNING", r"{bench_name}-redis:{bench_name}-redis-cache[\s]+RUNNING", r"{bench_name}-redis:{bench_name}-redis-queue[\s]+RUNNING", - r"{bench_name}-redis:{bench_name}-redis-socketio[\s]+RUNNING", ] if use_rq: From 5a2c052b9bee0a7d7e6247ed7e07d83274662960 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 7 Aug 2023 14:28:02 +0530 Subject: [PATCH 3/3] feat: Multi queue consumption (#1475) Ref: https://frappe.io/blog/engineering/reducing-memory-footprint-of-frappe-framework --- bench/config/supervisor.py | 39 +++++++++++----- bench/config/templates/supervisor.conf | 62 ++++---------------------- 2 files changed, 35 insertions(+), 66 deletions(-) diff --git a/bench/config/supervisor.py b/bench/config/supervisor.py index 7b482fa5..8e64bdb5 100644 --- a/bench/config/supervisor.py +++ b/bench/config/supervisor.py @@ -3,21 +3,20 @@ import getpass import logging import os -# imports - module imports -import bench -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, - get_default_max_requests, - compute_max_requests_jitter, -) - # imports - third party imports import click +# imports - module imports +import bench +from bench.app import use_rq +from bench.bench import Bench +from bench.config.common_site_config import ( + compute_max_requests_jitter, + get_default_max_requests, + get_gunicorn_workers, + update_config, +) +from bench.utils import get_bench_name, which logger = logging.getLogger(bench.PROJECT_NAME) @@ -58,6 +57,7 @@ def generate_supervisor_config(bench_path, user=None, yes=False, skip_redis=Fals "bench_cmd": which("bench"), "skip_redis": skip_redis, "workers": config.get("workers", {}), + "multi_queue_consumption": can_enable_multi_queue_consumption(bench_path), } ) @@ -90,6 +90,21 @@ def get_supervisord_conf(): return possibility +def can_enable_multi_queue_consumption(bench_path: str) -> bool: + try: + from semantic_version import Version + + from bench.utils.app import get_current_version + + supported_version = Version(major=14, minor=18, patch=0) + + frappe_version = Version(get_current_version("frappe", bench_path=bench_path)) + + return frappe_version > supported_version + except Exception: + return False + + def check_supervisord_config(user=None): """From bench v5.x, we're moving to supervisor running as user""" # i don't think bench should be responsible for this but we're way past this now... diff --git a/bench/config/templates/supervisor.conf b/bench/config/templates/supervisor.conf index 240701c9..de5f0298 100644 --- a/bench/config/templates/supervisor.conf +++ b/bench/config/templates/supervisor.conf @@ -12,7 +12,6 @@ stderr_logfile={{ bench_dir }}/logs/web.error.log user={{ user }} directory={{ sites_dir }} -{% if use_rq %} [program:{{ bench_name }}-frappe-schedule] command={{ bench_cmd }} schedule priority=3 @@ -23,6 +22,7 @@ stderr_logfile={{ bench_dir }}/logs/schedule.error.log user={{ user }} directory={{ bench_dir }} +{% if not multi_queue_consumption %} [program:{{ bench_name }}-frappe-default-worker] command={{ bench_cmd }} worker --queue default priority=4 @@ -36,9 +36,10 @@ directory={{ bench_dir }} killasgroup=true numprocs={{ background_workers }} process_name=%(program_name)s-%(process_num)d +{% endif %} [program:{{ bench_name }}-frappe-short-worker] -command={{ bench_cmd }} worker --queue short +command={{ bench_cmd }} worker --queue short{{',default' if multi_queue_consumption else ''}} priority=4 autostart=true autorestart=true @@ -52,7 +53,7 @@ numprocs={{ background_workers }} process_name=%(program_name)s-%(process_num)d [program:{{ bench_name }}-frappe-long-worker] -command={{ bench_cmd }} worker --queue long +command={{ bench_cmd }} worker --queue long{{',default,short' if multi_queue_consumption else ''}} priority=4 autostart=true autorestart=true @@ -81,54 +82,6 @@ numprocs={{ worker_details["background_workers"] or background_workers }} process_name=%(program_name)s-%(process_num)d {% endfor %} -{% else %} -[program:{{ bench_name }}-frappe-workerbeat] -command={{ bench_dir }}/env/bin/python -m frappe.celery_app beat -s beat.schedule -priority=3 -autostart=true -autorestart=true -stdout_logfile={{ bench_dir }}/logs/workerbeat.log -stderr_logfile={{ bench_dir }}/logs/workerbeat.error.log -user={{ user }} -directory={{ sites_dir }} - -[program:{{ bench_name }}-frappe-worker] -command={{ bench_dir }}/env/bin/python -m frappe.celery_app worker -n jobs@%%h -Ofair --soft-time-limit 360 --time-limit 390 --loglevel INFO -priority=4 -autostart=true -autorestart=true -stdout_logfile={{ bench_dir }}/logs/worker.log -stderr_logfile={{ bench_dir }}/logs/worker.error.log -user={{ user }} -stopwaitsecs=400 -directory={{ sites_dir }} -killasgroup=true - -[program:{{ bench_name }}-frappe-longjob-worker] -command={{ bench_dir }}/env/bin/python -m frappe.celery_app worker -n longjobs@%%h -Ofair --soft-time-limit 1500 --time-limit 1530 --loglevel INFO -priority=2 -autostart=true -autorestart=true -stdout_logfile={{ bench_dir }}/logs/worker.log -stderr_logfile={{ bench_dir }}/logs/worker.error.log -user={{ user }} -stopwaitsecs=1540 -directory={{ sites_dir }} -killasgroup=true - -[program:{{ bench_name }}-frappe-async-worker] -command={{ bench_dir }}/env/bin/python -m frappe.celery_app worker -n async@%%h -Ofair --soft-time-limit 1500 --time-limit 1530 --loglevel INFO -priority=2 -autostart=true -autorestart=true -stdout_logfile={{ bench_dir }}/logs/worker.log -stderr_logfile={{ bench_dir }}/logs/worker.error.log -user={{ user }} -stopwaitsecs=1540 -directory={{ sites_dir }} -killasgroup=true - -{% endif %} {% if not skip_redis %} [program:{{ bench_name }}-redis-cache] @@ -167,15 +120,16 @@ directory={{ bench_dir }} [group:{{ bench_name }}-web] programs={{ bench_name }}-frappe-web {%- if node -%} ,{{ bench_name }}-node-socketio {%- endif%} -{% if use_rq %} + +{% if multi_queue_consumption %} [group:{{ bench_name }}-workers] -programs={{ bench_name }}-frappe-schedule,{{ bench_name }}-frappe-default-worker,{{ bench_name }}-frappe-short-worker,{{ bench_name }}-frappe-long-worker{%- for worker_name in workers -%},{{ bench_name }}-frappe-{{ worker_name }}-worker{%- endfor %} +programs={{ bench_name }}-frappe-schedule,{{ bench_name }}-frappe-short-worker,{{ bench_name }}-frappe-long-worker{%- for worker_name in workers -%},{{ bench_name }}-frappe-{{ worker_name }}-worker{%- endfor %} {% else %} [group:{{ bench_name }}-workers] -programs={{ bench_name }}-frappe-workerbeat,{{ bench_name }}-frappe-worker,{{ bench_name }}-frappe-longjob-worker,{{ bench_name }}-frappe-async-worker{%- for worker_name in workers -%},{{ bench_name }}-frappe-{{ worker_name }}-worker{%- endfor %} +programs={{ bench_name }}-frappe-schedule,{{ bench_name }}-frappe-default-worker,{{ bench_name }}-frappe-short-worker,{{ bench_name }}-frappe-long-worker{%- for worker_name in workers -%},{{ bench_name }}-frappe-{{ worker_name }}-worker{%- endfor %} {% endif %}