From d51272cc3db9fa6de961adc7f651821d9291f916 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 11 Jun 2021 13:58:09 -0700 Subject: [PATCH] puppet: Remove zulip_deliver_scheduled_* from zulip-workers:*. Staging and other hosts that are `zulip::app_frontend_base` but not `zulip::app_frontend_once` do not have a /etc/supervisor/conf.d/zulip/zulip-once.conf and as such do not have `zulip_deliver_scheduled_emails` or `zulip_deliver_scheduled_messages` and thus supervisor will fail to reload. Making the contents of `zulip-workers` contingent on if the server is _also_ a `-once` server is complicated, and would involve using Concat fragments, which severely limit readability. Instead, expel those two from `zulip-workers`; this is somewhat reasonable, since they are use an entirely different codepath from zulip_events_*, using the database rather than RabbitMQ for their queuing. --- .../zulip_app_frontend/check_email_deliverer_process | 2 +- puppet/zulip/templates/supervisor/zulip.conf.template.erb | 4 ++-- scripts/lib/zulip_tools.py | 4 +++- scripts/refresh-sharding-and-restart | 3 +++ scripts/restart-server | 8 ++++++++ scripts/stop-server | 7 +++++++ 6 files changed, 24 insertions(+), 4 deletions(-) diff --git a/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_email_deliverer_process b/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_email_deliverer_process index 6754519337..808c2b31b3 100755 --- a/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_email_deliverer_process +++ b/puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_email_deliverer_process @@ -5,7 +5,7 @@ #"CRITICAL": 2 #"UNKNOWN": 3 -SUPERVISOR_STATUS=$(supervisorctl status zulip-workers:zulip_deliver_scheduled_emails 2>&1) +SUPERVISOR_STATUS=$(supervisorctl status zulip_deliver_scheduled_emails 2>&1) STATUS=$(echo "$SUPERVISOR_STATUS" | awk '{ print $2 }') case "$STATUS" in diff --git a/puppet/zulip/templates/supervisor/zulip.conf.template.erb b/puppet/zulip/templates/supervisor/zulip.conf.template.erb index 3516744e5b..1b5a00103b 100644 --- a/puppet/zulip/templates/supervisor/zulip.conf.template.erb +++ b/puppet/zulip/templates/supervisor/zulip.conf.template.erb @@ -101,9 +101,9 @@ killasgroup=true ; Without this, we leak processes every restart [group:zulip-workers] <% if @queues_multiprocess %> ; each refers to 'x' in [program:x] definitions -programs=zulip_deliver_scheduled_emails, zulip_deliver_scheduled_messages, <% @queues.each_with_index do |queue, i| -%>zulip_events_<%= queue %><%= ',' if i < (@queues.size - 1) %> <% end -%> +programs=<% @queues.each_with_index do |queue, i| -%>zulip_events_<%= queue %><%= ',' if i < (@queues.size - 1) %> <% end -%> <% else %> -programs=zulip_deliver_scheduled_emails, zulip_events, zulip_deliver_scheduled_messages +programs=zulip_events <% end %> ; The [include] section can just contain the "files" setting. This diff --git a/scripts/lib/zulip_tools.py b/scripts/lib/zulip_tools.py index c7c2cba3e8..294c5ee349 100755 --- a/scripts/lib/zulip_tools.py +++ b/scripts/lib/zulip_tools.py @@ -621,7 +621,9 @@ def is_vagrant_env_host(path: str) -> bool: return ".vagrant" in os.listdir(path) -def has_application_server() -> bool: +def has_application_server(once: bool = False) -> bool: + if once: + return os.path.exists("/etc/supervisor/conf.d/zulip/zulip-once.conf") return ( # Current path os.path.exists("/etc/supervisor/conf.d/zulip/zulip.conf") diff --git a/scripts/refresh-sharding-and-restart b/scripts/refresh-sharding-and-restart index ce43c76ead..aae70674ae 100755 --- a/scripts/refresh-sharding-and-restart +++ b/scripts/refresh-sharding-and-restart @@ -34,4 +34,7 @@ mv /etc/zulip/sharding.json.tmp /etc/zulip/sharding.json # reload nginx only after Django. supervisorctl restart zulip-django supervisorctl restart zulip-workers:* +if [ -f /etc/supervisor/conf.d/zulip/zulip-once.conf ]; then + supervisorctl restart zulip_deliver_scheduled_emails zulip_deliver_scheduled_messages +fi service nginx reload diff --git a/scripts/restart-server b/scripts/restart-server index d3a489eca1..359ff95651 100755 --- a/scripts/restart-server +++ b/scripts/restart-server @@ -91,6 +91,14 @@ if has_application_server(): worker_status.check_returncode() workers.extend(status_line.split()[0] for status_line in worker_status.stdout.splitlines()) + if has_application_server(once=True): + workers.extend( + [ + "zulip_deliver_scheduled_emails", + "zulip_deliver_scheduled_messages", + ] + ) + if has_process_fts_updates(): workers.append("process-fts-updates") diff --git a/scripts/stop-server b/scripts/stop-server index 0588403d99..5cf9f7f239 100755 --- a/scripts/stop-server +++ b/scripts/stop-server @@ -40,6 +40,13 @@ if has_application_server(): services.append("zulip-django") services.extend(["zulip-tornado", "zulip-tornado:*"]) services.append("zulip-workers:*") + if has_application_server(once=True): + services.extend( + [ + "zulip_deliver_scheduled_emails", + "zulip_deliver_scheduled_messages", + ] + ) subprocess.check_call(["supervisorctl", "stop", *services])