From 82eacb0e071657e796952638fe8da90cdb94f2a1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 24 May 2021 14:03:30 +0100 Subject: Fix --no-daemonize for synctl with workers (#9995) --- changelog.d/9995.bugfix | 1 + synctl | 102 +++++++++++++++--------------------------------- 2 files changed, 33 insertions(+), 70 deletions(-) create mode 100644 changelog.d/9995.bugfix diff --git a/changelog.d/9995.bugfix b/changelog.d/9995.bugfix new file mode 100644 index 0000000000..3b63e7c42a --- /dev/null +++ b/changelog.d/9995.bugfix @@ -0,0 +1 @@ +Fix `synctl`'s `--no-daemonize` parameter to work correctly with worker processes. diff --git a/synctl b/synctl index ccf404accb..6ce19918d2 100755 --- a/synctl +++ b/synctl @@ -24,12 +24,13 @@ import signal import subprocess import sys import time +from typing import Iterable import yaml from synapse.config import find_config_files -SYNAPSE = [sys.executable, "-m", "synapse.app.homeserver"] +MAIN_PROCESS = "synapse.app.homeserver" GREEN = "\x1b[1;32m" YELLOW = "\x1b[1;33m" @@ -68,71 +69,37 @@ def abort(message, colour=RED, stream=sys.stderr): sys.exit(1) -def start(configfile: str, daemonize: bool = True) -> bool: - """Attempts to start synapse. +def start(pidfile: str, app: str, config_files: Iterable[str], daemonize: bool) -> bool: + """Attempts to start a synapse main or worker process. Args: - configfile: path to a yaml synapse config file - daemonize: whether to daemonize synapse or keep it attached to the current - session + pidfile: the pidfile we expect the process to create + app: the python module to run + config_files: config files to pass to synapse + daemonize: if True, will include a --daemonize argument to synapse Returns: - True if the process started successfully + True if the process started successfully or was already running False if there was an error starting the process - - If deamonize is False it will only return once synapse exits. """ - write("Starting ...") - args = SYNAPSE - - if daemonize: - args.extend(["--daemonize", "-c", configfile]) - else: - args.extend(["-c", configfile]) - - try: - subprocess.check_call(args) - write("started synapse.app.homeserver(%r)" % (configfile,), colour=GREEN) + if os.path.exists(pidfile) and pid_running(int(open(pidfile).read())): + print(app + " already running") return True - except subprocess.CalledProcessError as e: - write( - "error starting (exit code: %d); see above for logs" % e.returncode, - colour=RED, - ) - return False - -def start_worker(app: str, configfile: str, worker_configfile: str) -> bool: - """Attempts to start a synapse worker. - Args: - app: name of the worker's appservice - configfile: path to a yaml synapse config file - worker_configfile: path to worker specific yaml synapse file - - Returns: - True if the process started successfully - False if there was an error starting the process - """ - - args = [ - sys.executable, - "-m", - app, - "-c", - configfile, - "-c", - worker_configfile, - "--daemonize", - ] + args = [sys.executable, "-m", app] + for c in config_files: + args += ["-c", c] + if daemonize: + args.append("--daemonize") try: subprocess.check_call(args) - write("started %s(%r)" % (app, worker_configfile), colour=GREEN) + write("started %s(%s)" % (app, ",".join(config_files)), colour=GREEN) return True except subprocess.CalledProcessError as e: write( - "error starting %s(%r) (exit code: %d); see above for logs" - % (app, worker_configfile, e.returncode), + "error starting %s(%s) (exit code: %d); see above for logs" + % (app, ",".join(config_files), e.returncode), colour=RED, ) return False @@ -224,10 +191,11 @@ def main(): if not os.path.exists(configfile): write( - "No config file found\n" - "To generate a config file, run '%s -c %s --generate-config" - " --server-name= --report-stats='\n" - % (" ".join(SYNAPSE), options.configfile), + f"Config file {configfile} does not exist.\n" + f"To generate a config file, run:\n" + f" {sys.executable} -m {MAIN_PROCESS}" + f" -c {configfile} --generate-config" + f" --server-name= --report-stats=\n", stream=sys.stderr, ) sys.exit(1) @@ -323,7 +291,7 @@ def main(): has_stopped = False if start_stop_synapse: - if not stop(pidfile, "synapse.app.homeserver"): + if not stop(pidfile, MAIN_PROCESS): has_stopped = False if not has_stopped and action == "stop": sys.exit(1) @@ -346,30 +314,24 @@ def main(): if action == "start" or action == "restart": error = False if start_stop_synapse: - # Check if synapse is already running - if os.path.exists(pidfile) and pid_running(int(open(pidfile).read())): - abort("synapse.app.homeserver already running") - - if not start(configfile, bool(options.daemonize)): + if not start(pidfile, MAIN_PROCESS, (configfile,), options.daemonize): error = True for worker in workers: env = os.environ.copy() - # Skip starting a worker if its already running - if os.path.exists(worker.pidfile) and pid_running( - int(open(worker.pidfile).read()) - ): - print(worker.app + " already running") - continue - if worker.cache_factor: os.environ["SYNAPSE_CACHE_FACTOR"] = str(worker.cache_factor) for cache_name, factor in worker.cache_factors.items(): os.environ["SYNAPSE_CACHE_FACTOR_" + cache_name.upper()] = str(factor) - if not start_worker(worker.app, configfile, worker.configfile): + if not start( + worker.pidfile, + worker.app, + (configfile, worker.configfile), + options.daemonize, + ): error = True # Reset env back to the original -- cgit 1.4.1