From aa22e2f8fc40054c2b4b4d7f41591ff2abb87722 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 16 Aug 2021 17:50:47 +0100 Subject: [PATCH] Move update_user_dir from worker type / config option to config option only Also clean up the worker sample configuration section a little bit. --- docs/sample_config.yaml | 15 +++++++--- synapse/app/admin_cmd.py | 2 +- synapse/app/generic_worker.py | 16 ----------- synapse/config/server.py | 4 --- synapse/config/workers.py | 28 ++++++++++++++++--- synapse/handlers/user_directory.py | 2 +- synapse/rest/client/v2_alpha/shared_rooms.py | 2 +- tests/handlers/test_user_directory.py | 2 -- .../rest/client/v2_alpha/test_shared_rooms.py | 1 - tests/utils.py | 3 -- 10 files changed, 38 insertions(+), 37 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 7f94911dbf..b1e52913f8 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -2761,16 +2761,23 @@ opentracing: # events: worker1 # typing: worker1 -# The worker that is used to run background tasks (e.g. cleaning up expired -# data). If not provided this defaults to the main process. +# The name of the worker that is used to run background tasks (e.g. cleaning +# up expired data). If not provided this defaults to the main process. # #run_background_tasks_on: worker1 -# The worker that is used to notify application services of new traffic within -# their configured namespace. If not provided this defaults to the main process. +# The name of the worker that is used to notify application services of new +# traffic within their configured namespace. If not provided this defaults +# to the main process. # #notify_appservices_from_worker: worker2 +# The name of the worker that is used to update the user directory tables as +# users are created, update their room memberships as well as their profiles. +# If not provided this defaults to the main process. +# +#update_user_directory_on_worker: worker3 + # A shared secret used by the replication APIs to authenticate HTTP requests # from workers. # diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index 3234d9ebba..835ec057bd 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -197,7 +197,7 @@ def start(config_options): config.no_redirect_stdio = True # Explicitly disable background processes - config.update_user_directory = False + config.worker.should_update_user_directory = False config.run_background_tasks = False config.start_pushers = False config.pusher_shard_config.instances = [] diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 080f3e039d..308ea72ab9 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -427,22 +427,6 @@ def start(config_options): "synapse.app.user_dir", ) - if config.worker_app == "synapse.app.user_dir": - if config.server.update_user_directory: - sys.stderr.write( - "\nThe update_user_directory must be disabled in the main synapse process" - "\nbefore they can be run in a separate worker." - "\nPlease add ``update_user_directory: false`` to the main config" - "\n" - ) - sys.exit(1) - - # Force the pushers to start since they will be disabled in the main config - config.server.update_user_directory = True - else: - # For other worker types we force this to off. - config.server.update_user_directory = False - synapse.events.USE_FROZEN_DICTS = config.use_frozen_dicts synapse.util.caches.TRACK_MEMORY_USAGE = config.caches.track_memory_usage diff --git a/synapse/config/server.py b/synapse/config/server.py index 187b4301a0..e89670b96c 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -257,10 +257,6 @@ class ServerConfig(Config): self.presence_router_config, ) = load_module(presence_router_config, ("presence", "presence_router")) - # Whether to update the user directory or not. This should be set to - # false only if we are updating the user directory in a worker - self.update_user_directory = config.get("update_user_directory", True) - # whether to enable the media repository endpoints. This should be set # to false if the media repository is running as a separate endpoint; # doing so ensures that we will not run cache cleanup jobs on the diff --git a/synapse/config/workers.py b/synapse/config/workers.py index f6679e4466..d08f8dd830 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -295,6 +295,19 @@ class WorkerConfig(Config): self.worker_name is None and notify_appservices_instance == "master" ) or self.worker_name == notify_appservices_instance + # Whether this worker should update the user directory tables. + # + # As a note for developers, this task is currently not shardable, and thus should + # only be handled by a single process. + # + # No effort is made here to ensure only a single instance of this task running. + update_user_directory_instance = ( + config.get("update_user_directory_on_worker") or "master" + ) + self.should_update_user_directory = ( + self.worker_name is None and update_user_directory_instance == "master" + ) or self.worker_name == update_user_directory_instance + def generate_config_section(self, config_dir_path, server_name, **kwargs): return """\ ## Workers ## @@ -331,16 +344,23 @@ class WorkerConfig(Config): # events: worker1 # typing: worker1 - # The worker that is used to run background tasks (e.g. cleaning up expired - # data). If not provided this defaults to the main process. + # The name of the worker that is used to run background tasks (e.g. cleaning + # up expired data). If not provided this defaults to the main process. # #run_background_tasks_on: worker1 - # The worker that is used to notify application services of new traffic within - # their configured namespace. If not provided this defaults to the main process. + # The name of the worker that is used to notify application services of new + # traffic within their configured namespace. If not provided this defaults + # to the main process. # #notify_appservices_from_worker: worker2 + # The name of the worker that is used to update the user directory tables as + # users are created, update their room memberships as well as their profiles. + # If not provided this defaults to the main process. + # + #update_user_directory_on_worker: worker3 + # A shared secret used by the replication APIs to authenticate HTTP requests # from workers. # diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 6edb1da50a..05c1ee30ed 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -48,7 +48,7 @@ class UserDirectoryHandler(StateDeltasHandler): self.clock = hs.get_clock() self.notifier = hs.get_notifier() self.is_mine_id = hs.is_mine_id - self.update_user_directory = hs.config.update_user_directory + self.update_user_directory = hs.config.worker.should_update_user_directory self.search_all_users = hs.config.user_directory_search_all_users self.spam_checker = hs.get_spam_checker() # The current position in the current_state_delta stream diff --git a/synapse/rest/client/v2_alpha/shared_rooms.py b/synapse/rest/client/v2_alpha/shared_rooms.py index d2e7f04b40..fb8a57298d 100644 --- a/synapse/rest/client/v2_alpha/shared_rooms.py +++ b/synapse/rest/client/v2_alpha/shared_rooms.py @@ -36,7 +36,7 @@ class UserSharedRoomsServlet(RestServlet): super().__init__() self.auth = hs.get_auth() self.store = hs.get_datastore() - self.user_directory_active = hs.config.update_user_directory + self.user_directory_active = hs.config.worker.should_update_user_directory async def on_GET(self, request, user_id): diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 549876dc85..823d768570 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -40,7 +40,6 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() - config["update_user_directory"] = True return self.setup_test_homeserver(config=config) def prepare(self, reactor, clock, hs): @@ -653,7 +652,6 @@ class TestUserDirSearchDisabled(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() - config["update_user_directory"] = True hs = self.setup_test_homeserver(config=config) self.config = hs.config diff --git a/tests/rest/client/v2_alpha/test_shared_rooms.py b/tests/rest/client/v2_alpha/test_shared_rooms.py index cedb9614a8..84881fafd5 100644 --- a/tests/rest/client/v2_alpha/test_shared_rooms.py +++ b/tests/rest/client/v2_alpha/test_shared_rooms.py @@ -33,7 +33,6 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() - config["update_user_directory"] = True return self.setup_test_homeserver(config=config) def prepare(self, reactor, clock, hs): diff --git a/tests/utils.py b/tests/utils.py index f3458ca88d..56e75ea926 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -168,9 +168,6 @@ def default_config(name, parse=False): # We need a sane default_room_version, otherwise attempts to create # rooms will fail. "default_room_version": DEFAULT_ROOM_VERSION, - # disable user directory updates, because they get done in the - # background, which upsets the test runner. - "update_user_directory": False, "caches": {"global_factor": 1}, "listeners": [{"port": 0, "type": "http"}], }