From 9a3d3b6500d0cac06f1aca840358692416f9fd2b Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:01:48 +0100 Subject: [PATCH 01/25] shift to worker_to_update_user_directory --- docker/configure_workers_and_start.py | 4 ++-- docs/sample_config.yaml | 8 ++++++++ docs/workers.md | 9 +++++---- synapse/app/generic_worker.py | 16 +++++----------- synapse/config/server.py | 6 +++--- synapse/handlers/user_directory.py | 2 +- synapse/rest/client/shared_rooms.py | 8 -------- synapse/server.py | 7 +++++++ tests/handlers/test_user_directory.py | 4 ++-- tests/rest/client/test_shared_rooms.py | 2 +- tests/utils.py | 2 +- 11 files changed, 35 insertions(+), 33 deletions(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index adbb551cee7f..dd6b04ac1414 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -50,8 +50,8 @@ "endpoint_patterns": [ "^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$" ], - "shared_extra_conf": {"update_user_directory": False}, - "worker_extra_conf": "", + "shared_extra_conf": {"worker_to_update_user_directory": "user_dir"}, + "worker_extra_conf": {"worker_name": "user_dir"}, }, "media_repository": { "app": "synapse.app.media_repository", diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index ae476d19ac8e..f2217f31a635 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -204,6 +204,14 @@ presence: # #enable_search: false +# User Directory Updates +# +# If specified, a worker with that name will be the only one able to update +# the user directory. Important for querying shared rooms. If the worker is +# specified, but not running, the user directory may become outdated. +# Defaults to null, meaning the main process will handle this. +#worker_to_update_user_directory: null + # Prevent outgoing requests from being sent to the following blacklisted IP address # CIDR ranges. If this option is not specified then it defaults to private IP # address ranges (see the example below). diff --git a/docs/workers.md b/docs/workers.md index 17c8bfeef6e2..d3d216bc8307 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -374,7 +374,7 @@ the shared configuration would include: run_background_tasks_on: background_worker ``` -You might also wish to investigate the `update_user_directory` and +You might also wish to investigate the `worker_to_update_user_directory` and `media_instance_running_background_jobs` settings. ### `synapse.app.pusher` @@ -467,9 +467,10 @@ the following regular expressions: ^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$ -When using this worker you must also set `update_user_directory: False` in the -shared configuration file to stop the main synapse running background -jobs related to updating the user directory. +When using this worker you must also set `worker_to_update_user_directory` in the +shared configuration file to the user directory worker's name to stop the main +synapse running background jobs related to updating the user directory, and give +the user directory worker the exclusive right to update it instead. ### `synapse.app.frontend_proxy` diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index b4bed5bf40b9..855bbbb543a5 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -458,21 +458,15 @@ def start(config_options: List[str]) -> None: config.appservice.notify_appservices = False if config.worker.worker_app == "synapse.app.user_dir": - if config.server.update_user_directory: + if config.worker.worker_name != config.server.worker_to_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" + "\nThe worker_to_update_user_directory config variable must point to this worker's name" + "\nbefore this worker is able to run it." + "\nPlease add or edit " + f"``worker_to_update_user_directory: \"{config.worker.worker_name}\"`` in 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.server.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 8445e9dd0509..2b1e40b40f46 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -321,9 +321,9 @@ def read_config(self, config, **kwargs): 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) + # Which worker is responsible for updating the user directory, + # None means the main process handles this. + self.worker_to_update_user_directory = config.get("worker_to_update_user_directory", None) # whether to enable the media repository endpoints. This should be set # to false if the media repository is running as a separate endpoint; diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index a0eb45446f56..e2632859b54a 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -60,7 +60,7 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self.notifier = hs.get_notifier() self.is_mine_id = hs.is_mine_id - self.update_user_directory = hs.config.server.update_user_directory + self.update_user_directory = hs.should_update_user_directory() self.search_all_users = hs.config.userdirectory.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/shared_rooms.py b/synapse/rest/client/shared_rooms.py index 09a46737de06..f7970f00065e 100644 --- a/synapse/rest/client/shared_rooms.py +++ b/synapse/rest/client/shared_rooms.py @@ -42,19 +42,11 @@ def __init__(self, hs: "HomeServer"): super().__init__() self.auth = hs.get_auth() self.store = hs.get_datastore() - self.user_directory_active = hs.config.server.update_user_directory async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: - if not self.user_directory_active: - raise SynapseError( - code=400, - msg="The user directory is disabled on this server. Cannot determine shared rooms.", - errcode=Codes.FORBIDDEN, - ) - UserID.from_string(user_id) requester = await self.auth.get_user_by_req(request) diff --git a/synapse/server.py b/synapse/server.py index 877eba6c0803..0bee796947e0 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -834,6 +834,13 @@ def should_send_federation(self) -> bool: "Should this server be sending federation traffic directly?" return self.config.worker.send_federation + def should_update_user_directory(self) -> bool: + "Should this process be updating the user directory?" + if self.config.worker.worker_app is None: # we're the main process + return self.config.server.worker_to_update_user_directory is None + else: + return self.config.server.worker_to_update_user_directory == self.config.worker.worker_name + @cache_in_self def get_request_ratelimiter(self) -> RequestRatelimiter: return RequestRatelimiter( diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 70c621b825f4..0bbb6acd1e83 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -56,7 +56,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() - config["update_user_directory"] = True + config["worker_to_update_user_directory"] = None self.appservice = ApplicationService( token="i_am_an_app_service", @@ -1014,7 +1014,7 @@ class TestUserDirSearchDisabled(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() - config["update_user_directory"] = True + config["worker_to_update_user_directory"] = None hs = self.setup_test_homeserver(config=config) self.config = hs.config diff --git a/tests/rest/client/test_shared_rooms.py b/tests/rest/client/test_shared_rooms.py index 283eccd53f95..caa3554c6f88 100644 --- a/tests/rest/client/test_shared_rooms.py +++ b/tests/rest/client/test_shared_rooms.py @@ -32,7 +32,7 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() - config["update_user_directory"] = True + config["worker_to_update_user_directory"] = None return self.setup_test_homeserver(config=config) def prepare(self, reactor, clock, hs): diff --git a/tests/utils.py b/tests/utils.py index 983859120f55..1b5e8aabc1aa 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -169,7 +169,7 @@ def default_config(name, parse=False): "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, + "worker_to_update_user_directory": "dingleberry", "caches": {"global_factor": 1}, "listeners": [{"port": 0, "type": "http"}], } From 5f6bce72d1c55830d30ac56ef762c7b1f681ea7b Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:06:38 +0100 Subject: [PATCH 02/25] linter --- synapse/app/generic_worker.py | 2 +- synapse/config/server.py | 4 +++- synapse/server.py | 5 ++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 855bbbb543a5..e7e6330c5922 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -463,7 +463,7 @@ def start(config_options: List[str]) -> None: "\nThe worker_to_update_user_directory config variable must point to this worker's name" "\nbefore this worker is able to run it." "\nPlease add or edit " - f"``worker_to_update_user_directory: \"{config.worker.worker_name}\"`` in the main config\n" + f'``worker_to_update_user_directory: "{config.worker.worker_name}"`` in the main config\n' ) sys.exit(1) diff --git a/synapse/config/server.py b/synapse/config/server.py index 2b1e40b40f46..2d2b1ad93f3a 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -323,7 +323,9 @@ def read_config(self, config, **kwargs): # Which worker is responsible for updating the user directory, # None means the main process handles this. - self.worker_to_update_user_directory = config.get("worker_to_update_user_directory", None) + self.worker_to_update_user_directory = config.get( + "worker_to_update_user_directory", None + ) # whether to enable the media repository endpoints. This should be set # to false if the media repository is running as a separate endpoint; diff --git a/synapse/server.py b/synapse/server.py index 0bee796947e0..5737d5748637 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -839,7 +839,10 @@ def should_update_user_directory(self) -> bool: if self.config.worker.worker_app is None: # we're the main process return self.config.server.worker_to_update_user_directory is None else: - return self.config.server.worker_to_update_user_directory == self.config.worker.worker_name + return ( + self.config.server.worker_to_update_user_directory + == self.config.worker.worker_name + ) @cache_in_self def get_request_ratelimiter(self) -> RequestRatelimiter: From f24f1515220c1365237eb65ebddd18f95fb75433 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:08:05 +0100 Subject: [PATCH 03/25] news --- changelog.d/11450.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11450.feature diff --git a/changelog.d/11450.feature b/changelog.d/11450.feature new file mode 100644 index 000000000000..79333480c2ca --- /dev/null +++ b/changelog.d/11450.feature @@ -0,0 +1 @@ +Introduce `worker_to_update_user_directory` config variable. \ No newline at end of file From 0f05a6d645424953ad4279aa23cba798eef5d02e Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:13:19 +0100 Subject: [PATCH 04/25] update nonsensical name in default_config() to be more inspired --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 1b5e8aabc1aa..6c28a88e6df5 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -169,7 +169,7 @@ def default_config(name, parse=False): "default_room_version": DEFAULT_ROOM_VERSION, # disable user directory updates, because they get done in the # background, which upsets the test runner. - "worker_to_update_user_directory": "dingleberry", + "worker_to_update_user_directory": "B. F. Bugleberry", "caches": {"global_factor": 1}, "listeners": [{"port": 0, "type": "http"}], } From 300b51e491f0f7eae06b8f9720120aac099a1d86 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:21:54 +0100 Subject: [PATCH 05/25] add updated sample config --- docs/sample_config.yaml | 8 ++++++++ synapse/config/server.py | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index f2217f31a635..a4fd640d2004 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -142,6 +142,14 @@ presence: # #limit_profile_requests_to_users_who_share_rooms: true +# User Directory Updates +# +# If specified, a worker with that name will be the only one able to update +# the user directory. Important for querying shared rooms. If the worker is +# specified, but not running, the user directory may become outdated. +# Defaults to null, meaning the main process will handle this. +#worker_to_update_user_directory: null + # Uncomment to prevent a user's profile data from being retrieved and # displayed in a room until they have joined it. By default, a user's # profile data is included in an invite event, regardless of the values diff --git a/synapse/config/server.py b/synapse/config/server.py index 2d2b1ad93f3a..138aa00820b1 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -861,6 +861,14 @@ def generate_config_section( # #limit_profile_requests_to_users_who_share_rooms: true + # User Directory Updates + # + # If specified, a worker with that name will be the only one able to update + # the user directory. Important for querying shared rooms. If the worker is + # specified, but not running, the user directory may become outdated. + # Defaults to null, meaning the main process will handle this. + #worker_to_update_user_directory: null + # Uncomment to prevent a user's profile data from being retrieved and # displayed in a room until they have joined it. By default, a user's # profile data is included in an invite event, regardless of the values From 28e255c6e190f33a79016ed80f1c84a200039f25 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:39:13 +0100 Subject: [PATCH 06/25] update sample config the sequel --- docs/sample_config.yaml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index a4fd640d2004..65aa05cc49ac 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -212,14 +212,6 @@ presence: # #enable_search: false -# User Directory Updates -# -# If specified, a worker with that name will be the only one able to update -# the user directory. Important for querying shared rooms. If the worker is -# specified, but not running, the user directory may become outdated. -# Defaults to null, meaning the main process will handle this. -#worker_to_update_user_directory: null - # Prevent outgoing requests from being sent to the following blacklisted IP address # CIDR ranges. If this option is not specified then it defaults to private IP # address ranges (see the example below). From 47134d184a76a149864c559ec99fdf4c82785578 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 19:58:57 +0100 Subject: [PATCH 07/25] fix mypy issue --- synapse/app/admin_cmd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index 42238f7f280b..ec1c1e1e8968 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -211,7 +211,7 @@ def start(config_options: List[str]) -> None: config.logging.no_redirect_stdio = True # Explicitly disable background processes - config.server.update_user_directory = False + config.server.worker_to_update_user_directory = "" config.worker.run_background_tasks = False config.worker.start_pushers = False config.worker.pusher_shard_config.instances = [] From 4d518bd79b2f73d5e2a3e38299a53a276825eb98 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 20:18:24 +0100 Subject: [PATCH 08/25] fix sytest --- docker/configure_workers_and_start.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index dd6b04ac1414..eb5a6d768435 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -51,7 +51,7 @@ "^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$" ], "shared_extra_conf": {"worker_to_update_user_directory": "user_dir"}, - "worker_extra_conf": {"worker_name": "user_dir"}, + "worker_extra_conf": 'worker_name: "user_dir"', }, "media_repository": { "app": "synapse.app.media_repository", From b67fe883e4e15adf0f9af2533cfecd5364bbfcb7 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 20:30:01 +0100 Subject: [PATCH 09/25] debug sytest failures --- docker/configure_workers_and_start.py | 2 +- synapse/app/generic_worker.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index eb5a6d768435..e36f85a6a204 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -51,7 +51,7 @@ "^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$" ], "shared_extra_conf": {"worker_to_update_user_directory": "user_dir"}, - "worker_extra_conf": 'worker_name: "user_dir"', + "worker_extra_conf": "", }, "media_repository": { "app": "synapse.app.media_repository", diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index e7e6330c5922..2eacb7f1a526 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -461,7 +461,8 @@ def start(config_options: List[str]) -> None: if config.worker.worker_name != config.server.worker_to_update_user_directory: sys.stderr.write( "\nThe worker_to_update_user_directory config variable must point to this worker's name" - "\nbefore this worker is able to run it." + "\nto give this worker exclusive access to run it. (It is currently pointed at " + f"{config.server.worker_to_update_user_directory:!r})" "\nPlease add or edit " f'``worker_to_update_user_directory: "{config.worker.worker_name}"`` in the main config\n' ) From 7ede21e2e490897fec5daff996e86e5c0f232132 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 29 Nov 2021 20:38:02 +0100 Subject: [PATCH 10/25] debug sytest failures mk 2 --- synapse/app/generic_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 2eacb7f1a526..f2def510e149 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -462,7 +462,7 @@ def start(config_options: List[str]) -> None: sys.stderr.write( "\nThe worker_to_update_user_directory config variable must point to this worker's name" "\nto give this worker exclusive access to run it. (It is currently pointed at " - f"{config.server.worker_to_update_user_directory:!r})" + f"{config.server.worker_to_update_user_directory!r})" "\nPlease add or edit " f'``worker_to_update_user_directory: "{config.worker.worker_name}"`` in the main config\n' ) From 434d108f7a3f511bce0a534c0c6133639f990185 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 6 Dec 2021 20:17:45 +0100 Subject: [PATCH 11/25] add backwards compatibility --- docker/configure_workers_and_start.py | 2 +- synapse/app/generic_worker.py | 7 +++++- synapse/config/server.py | 36 ++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index e36f85a6a204..61881d20d9c8 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -50,7 +50,7 @@ "endpoint_patterns": [ "^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$" ], - "shared_extra_conf": {"worker_to_update_user_directory": "user_dir"}, + "shared_extra_conf": {"worker_to_update_user_directory": "user_dir1"}, "worker_extra_conf": "", }, "media_repository": { diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index f2def510e149..bfd51565d888 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -458,7 +458,12 @@ def start(config_options: List[str]) -> None: config.appservice.notify_appservices = False if config.worker.worker_app == "synapse.app.user_dir": - if config.worker.worker_name != config.server.worker_to_update_user_directory: + from synapse.config.server import ANY_USER_DIRECTORY_WORKER + + if ( + config.worker.worker_name != config.server.worker_to_update_user_directory + and config.worker.worker_name is not ANY_USER_DIRECTORY_WORKER + ): sys.stderr.write( "\nThe worker_to_update_user_directory config variable must point to this worker's name" "\nto give this worker exclusive access to run it. (It is currently pointed at " diff --git a/synapse/config/server.py b/synapse/config/server.py index 138aa00820b1..2d8874729035 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -35,6 +35,14 @@ logger = logging.Logger(__name__) +# an object to pass to the "default" parameter, with which we can +# `is` against to be sure we have an undefined config option. +UNDEFINED = object() + +# an object to pass to self.worker_to_update_user_directory if +# update_user_directory was defined, is used to compare with 'is'. +ANY_USER_DIRECTORY_WORKER = object() + # by default, we attempt to listen on both '::' *and* '0.0.0.0' because some OSes # (Windows, macOS, other BSD/Linux where net.ipv6.bindv6only is set) will only listen # on IPv6 when '::' is set. @@ -324,9 +332,28 @@ def read_config(self, config, **kwargs): # Which worker is responsible for updating the user directory, # None means the main process handles this. self.worker_to_update_user_directory = config.get( - "worker_to_update_user_directory", None + "worker_to_update_user_directory", UNDEFINED ) + update_user_directory = config.get("update_user_directory", UNDEFINED) + + # Resolve backwards compat between update_user_directory (UUD) and + # worker_to_update_user_directory (WTUUD): + # - if WTUUD is defined, just use it + # - if UUD and WTUUD are undefined, assume WTUUD is None + # - if UUD is defined (and True), assume WTUUD is None + # - if UUD is defined (and False), set sentinel object so that user_dir + # workers will work normally. + if self.worker_to_update_user_directory is UNDEFINED: + if update_user_directory is UNDEFINED: + self.worker_to_update_user_directory = None + else: + logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) + if update_user_directory: + self.worker_to_update_user_directory = None + else: + self.worker_to_update_user_directory = ANY_USER_DIRECTORY_WORKER + # 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 @@ -1359,6 +1386,13 @@ def parse_listener_def(listener: Any) -> ListenerConfig: configuration. """ +USER_UPDATE_DIRECTORY_DEPRECATION_WARNING = """ +Synapse now uses 'worker_to_update_user_directory' over 'update_user_directory', +you have set 'update_user_directory', and while synapse will work in a backwards +compatible manner, it is suggested to change this value to use +'worker_to_update_user_directory' instead. +""" + def _warn_if_webclient_configured(listeners: Iterable[ListenerConfig]) -> None: for listener in listeners: From f2490b3f509691b4ae039232d267bdeb58636053 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 6 Dec 2021 20:31:52 +0100 Subject: [PATCH 12/25] update config with false behaviour --- docs/sample_config.yaml | 3 +++ synapse/config/server.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 65aa05cc49ac..e3e5d191746d 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -147,7 +147,10 @@ presence: # If specified, a worker with that name will be the only one able to update # the user directory. Important for querying shared rooms. If the worker is # specified, but not running, the user directory may become outdated. +# # Defaults to null, meaning the main process will handle this. +# +# Set to false to make sure no process will handle the user_directory. #worker_to_update_user_directory: null # Uncomment to prevent a user's profile data from being retrieved and diff --git a/synapse/config/server.py b/synapse/config/server.py index 2d8874729035..25da395fea66 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -893,7 +893,10 @@ def generate_config_section( # If specified, a worker with that name will be the only one able to update # the user directory. Important for querying shared rooms. If the worker is # specified, but not running, the user directory may become outdated. + # # Defaults to null, meaning the main process will handle this. + # + # Set to false to make sure no process will handle the user_directory. #worker_to_update_user_directory: null # Uncomment to prevent a user's profile data from being retrieved and From 4593e15229837636392235530878eb93a1087039 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 6 Dec 2021 20:35:03 +0100 Subject: [PATCH 13/25] whoops --- synapse/app/generic_worker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index bfd51565d888..a9f1c4dc5a51 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -462,7 +462,8 @@ def start(config_options: List[str]) -> None: if ( config.worker.worker_name != config.server.worker_to_update_user_directory - and config.worker.worker_name is not ANY_USER_DIRECTORY_WORKER + and config.server.worker_to_update_user_directory + is not ANY_USER_DIRECTORY_WORKER ): sys.stderr.write( "\nThe worker_to_update_user_directory config variable must point to this worker's name" From db6625e3346b29d22a5f24fec6fca42c6dbabbcc Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Tue, 7 Dec 2021 18:26:37 +0100 Subject: [PATCH 14/25] add special case for user directory workers --- synapse/server.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/server.py b/synapse/server.py index 5737d5748637..ac8484eed86e 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -836,8 +836,13 @@ def should_send_federation(self) -> bool: def should_update_user_directory(self) -> bool: "Should this process be updating the user directory?" + from synapse.config.server import ANY_USER_DIRECTORY_WORKER + if self.config.worker.worker_app is None: # we're the main process return self.config.server.worker_to_update_user_directory is None + elif self.config.server.worker_to_update_user_directory is ANY_USER_DIRECTORY_WORKER: + # A special backwards-compatible case for if update_user_directory is False + return self.config.worker.worker_app == "synapse.app.user_dir" else: return ( self.config.server.worker_to_update_user_directory From 6f6b83b5492f43dc8c389f843a6d4fc204bf2233 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Tue, 7 Dec 2021 18:31:01 +0100 Subject: [PATCH 15/25] the lint, it sees all, knows all --- synapse/server.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/server.py b/synapse/server.py index ac8484eed86e..8d7b9170d7c8 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -840,7 +840,10 @@ def should_update_user_directory(self) -> bool: if self.config.worker.worker_app is None: # we're the main process return self.config.server.worker_to_update_user_directory is None - elif self.config.server.worker_to_update_user_directory is ANY_USER_DIRECTORY_WORKER: + elif ( + self.config.server.worker_to_update_user_directory + is ANY_USER_DIRECTORY_WORKER + ): # A special backwards-compatible case for if update_user_directory is False return self.config.worker.worker_app == "synapse.app.user_dir" else: From 7808e86cd1d785a81dea1ce7cd849d3c4eb40001 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Wed, 8 Dec 2021 16:43:23 +0100 Subject: [PATCH 16/25] apply review feedback --- synapse/app/admin_cmd.py | 2 +- synapse/app/generic_worker.py | 4 +--- synapse/config/server.py | 10 +++++++++- synapse/server.py | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index ec1c1e1e8968..6c79a0bfc26b 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -211,7 +211,7 @@ def start(config_options: List[str]) -> None: config.logging.no_redirect_stdio = True # Explicitly disable background processes - config.server.worker_to_update_user_directory = "" + config.server.worker_to_update_user_directory = False config.worker.run_background_tasks = False config.worker.start_pushers = False config.worker.pusher_shard_config.instances = [] diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index a9f1c4dc5a51..782fcd646b8a 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -40,7 +40,7 @@ from synapse.config._base import ConfigError from synapse.config.homeserver import HomeServerConfig from synapse.config.logger import setup_logging -from synapse.config.server import ListenerConfig +from synapse.config.server import ANY_USER_DIRECTORY_WORKER, ListenerConfig from synapse.federation.transport.server import TransportLayerServer from synapse.http.server import JsonResource, OptionsResource from synapse.http.servlet import RestServlet, parse_json_object_from_request @@ -458,8 +458,6 @@ def start(config_options: List[str]) -> None: config.appservice.notify_appservices = False if config.worker.worker_app == "synapse.app.user_dir": - from synapse.config.server import ANY_USER_DIRECTORY_WORKER - if ( config.worker.worker_name != config.server.worker_to_update_user_directory and config.server.worker_to_update_user_directory diff --git a/synapse/config/server.py b/synapse/config/server.py index 25da395fea66..957c4b06331c 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -331,7 +331,12 @@ def read_config(self, config, **kwargs): # Which worker is responsible for updating the user directory, # None means the main process handles this. - self.worker_to_update_user_directory = config.get( + # Eventually this is expected to hold None, a `str`, or ANY_USER_DIRECTORY_WORKER + # But it is possible for a user to pass a non-string value here as well, such as False, + # in which case that'll not match with anything. + # Consider this an `Any`, only match positively with '== worker_name', 'is None' or + # 'is ANY_USER_DIRECTORY_WORKER' to determine if the local process should be updating the directory. + self.worker_to_update_user_directory: Any = config.get( "worker_to_update_user_directory", UNDEFINED ) @@ -354,6 +359,9 @@ def read_config(self, config, **kwargs): else: self.worker_to_update_user_directory = ANY_USER_DIRECTORY_WORKER + # Via all branches, this value is defined + assert self.worker_to_update_user_directory is not UNDEFINED + # 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/server.py b/synapse/server.py index 8d7b9170d7c8..d4638a96d3be 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -44,6 +44,7 @@ from synapse.appservice.api import ApplicationServiceApi from synapse.appservice.scheduler import ApplicationServiceScheduler from synapse.config.homeserver import HomeServerConfig +from synapse.config.server import ANY_USER_DIRECTORY_WORKER from synapse.crypto import context_factory from synapse.crypto.context_factory import RegularPolicyForHTTPS from synapse.crypto.keyring import Keyring @@ -836,7 +837,6 @@ def should_send_federation(self) -> bool: def should_update_user_directory(self) -> bool: "Should this process be updating the user directory?" - from synapse.config.server import ANY_USER_DIRECTORY_WORKER if self.config.worker.worker_app is None: # we're the main process return self.config.server.worker_to_update_user_directory is None From 2d2c9046aa45cc4173d93695337e5ffe25ae62de Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Thu, 9 Dec 2021 15:55:16 +0100 Subject: [PATCH 17/25] update name, move to workers.py --- changelog.d/11450.feature | 2 +- docker/configure_workers_and_start.py | 2 +- docs/sample_config.yaml | 17 +++----- docs/workers.md | 4 +- synapse/app/admin_cmd.py | 2 +- synapse/app/generic_worker.py | 13 +++--- synapse/config/server.py | 59 -------------------------- synapse/config/workers.py | 57 ++++++++++++++++++++++++- synapse/server.py | 11 ++--- tests/handlers/test_user_directory.py | 4 +- tests/rest/client/test_shared_rooms.py | 2 +- tests/utils.py | 2 +- 12 files changed, 81 insertions(+), 94 deletions(-) diff --git a/changelog.d/11450.feature b/changelog.d/11450.feature index 79333480c2ca..72114ede3ce8 100644 --- a/changelog.d/11450.feature +++ b/changelog.d/11450.feature @@ -1 +1 @@ -Introduce `worker_to_update_user_directory` config variable. \ No newline at end of file +Introduce `update_user_directory_on` config variable. \ No newline at end of file diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 61881d20d9c8..922f7d44547f 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -50,7 +50,7 @@ "endpoint_patterns": [ "^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$" ], - "shared_extra_conf": {"worker_to_update_user_directory": "user_dir1"}, + "shared_extra_conf": {"update_user_directory_on": "user_dir1"}, "worker_extra_conf": "", }, "media_repository": { diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index e3e5d191746d..18094d000cf8 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -142,17 +142,6 @@ presence: # #limit_profile_requests_to_users_who_share_rooms: true -# User Directory Updates -# -# If specified, a worker with that name will be the only one able to update -# the user directory. Important for querying shared rooms. If the worker is -# specified, but not running, the user directory may become outdated. -# -# Defaults to null, meaning the main process will handle this. -# -# Set to false to make sure no process will handle the user_directory. -#worker_to_update_user_directory: null - # Uncomment to prevent a user's profile data from being retrieved and # displayed in a room until they have joined it. By default, a user's # profile data is included in an invite event, regardless of the values @@ -2630,6 +2619,12 @@ opentracing: # #run_background_tasks_on: worker1 +# The worker that is used to run user directory update tasks +# (e.g. users in public rooms, shared rooms between users.). +# +# If not provided, or null, this defaults to the main process. +#update_user_directory_on: null + # A shared secret used by the replication APIs to authenticate HTTP requests # from workers. # diff --git a/docs/workers.md b/docs/workers.md index d3d216bc8307..e576f410e23e 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -374,7 +374,7 @@ the shared configuration would include: run_background_tasks_on: background_worker ``` -You might also wish to investigate the `worker_to_update_user_directory` and +You might also wish to investigate the `update_user_directory_on` and `media_instance_running_background_jobs` settings. ### `synapse.app.pusher` @@ -467,7 +467,7 @@ the following regular expressions: ^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$ -When using this worker you must also set `worker_to_update_user_directory` in the +When using this worker you must also set `update_user_directory_on` in the shared configuration file to the user directory worker's name to stop the main synapse running background jobs related to updating the user directory, and give the user directory worker the exclusive right to update it instead. diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index 6c79a0bfc26b..b4a190b13f1a 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -211,7 +211,7 @@ def start(config_options: List[str]) -> None: config.logging.no_redirect_stdio = True # Explicitly disable background processes - config.server.worker_to_update_user_directory = False + config.worker.update_user_directory_on = "" config.worker.run_background_tasks = False config.worker.start_pushers = False config.worker.pusher_shard_config.instances = [] diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 782fcd646b8a..9120207e9a3d 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -40,7 +40,7 @@ from synapse.config._base import ConfigError from synapse.config.homeserver import HomeServerConfig from synapse.config.logger import setup_logging -from synapse.config.server import ANY_USER_DIRECTORY_WORKER, ListenerConfig +from synapse.config.workers import ANY_USER_DIRECTORY_WORKER, ListenerConfig from synapse.federation.transport.server import TransportLayerServer from synapse.http.server import JsonResource, OptionsResource from synapse.http.servlet import RestServlet, parse_json_object_from_request @@ -459,16 +459,15 @@ def start(config_options: List[str]) -> None: if config.worker.worker_app == "synapse.app.user_dir": if ( - config.worker.worker_name != config.server.worker_to_update_user_directory - and config.server.worker_to_update_user_directory - is not ANY_USER_DIRECTORY_WORKER + config.worker.worker_name != config.worker.update_user_directory_on + and config.worker.update_user_directory_on is not ANY_USER_DIRECTORY_WORKER ): sys.stderr.write( - "\nThe worker_to_update_user_directory config variable must point to this worker's name" + "\nThe update_user_directory_on config variable must point to this worker's name" "\nto give this worker exclusive access to run it. (It is currently pointed at " - f"{config.server.worker_to_update_user_directory!r})" + f"{config.worker.update_user_directory_on!r})" "\nPlease add or edit " - f'``worker_to_update_user_directory: "{config.worker.worker_name}"`` in the main config\n' + f'``update_user_directory_on: "{config.worker.worker_name}"`` in the main config\n' ) sys.exit(1) diff --git a/synapse/config/server.py b/synapse/config/server.py index 957c4b06331c..96b8cf407c9c 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -35,14 +35,6 @@ logger = logging.Logger(__name__) -# an object to pass to the "default" parameter, with which we can -# `is` against to be sure we have an undefined config option. -UNDEFINED = object() - -# an object to pass to self.worker_to_update_user_directory if -# update_user_directory was defined, is used to compare with 'is'. -ANY_USER_DIRECTORY_WORKER = object() - # by default, we attempt to listen on both '::' *and* '0.0.0.0' because some OSes # (Windows, macOS, other BSD/Linux where net.ipv6.bindv6only is set) will only listen # on IPv6 when '::' is set. @@ -329,39 +321,6 @@ def read_config(self, config, **kwargs): self.presence_router_config, ) = load_module(presence_router_config, ("presence", "presence_router")) - # Which worker is responsible for updating the user directory, - # None means the main process handles this. - # Eventually this is expected to hold None, a `str`, or ANY_USER_DIRECTORY_WORKER - # But it is possible for a user to pass a non-string value here as well, such as False, - # in which case that'll not match with anything. - # Consider this an `Any`, only match positively with '== worker_name', 'is None' or - # 'is ANY_USER_DIRECTORY_WORKER' to determine if the local process should be updating the directory. - self.worker_to_update_user_directory: Any = config.get( - "worker_to_update_user_directory", UNDEFINED - ) - - update_user_directory = config.get("update_user_directory", UNDEFINED) - - # Resolve backwards compat between update_user_directory (UUD) and - # worker_to_update_user_directory (WTUUD): - # - if WTUUD is defined, just use it - # - if UUD and WTUUD are undefined, assume WTUUD is None - # - if UUD is defined (and True), assume WTUUD is None - # - if UUD is defined (and False), set sentinel object so that user_dir - # workers will work normally. - if self.worker_to_update_user_directory is UNDEFINED: - if update_user_directory is UNDEFINED: - self.worker_to_update_user_directory = None - else: - logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - if update_user_directory: - self.worker_to_update_user_directory = None - else: - self.worker_to_update_user_directory = ANY_USER_DIRECTORY_WORKER - - # Via all branches, this value is defined - assert self.worker_to_update_user_directory is not UNDEFINED - # 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 @@ -896,17 +855,6 @@ def generate_config_section( # #limit_profile_requests_to_users_who_share_rooms: true - # User Directory Updates - # - # If specified, a worker with that name will be the only one able to update - # the user directory. Important for querying shared rooms. If the worker is - # specified, but not running, the user directory may become outdated. - # - # Defaults to null, meaning the main process will handle this. - # - # Set to false to make sure no process will handle the user_directory. - #worker_to_update_user_directory: null - # Uncomment to prevent a user's profile data from being retrieved and # displayed in a room until they have joined it. By default, a user's # profile data is included in an invite event, regardless of the values @@ -1397,13 +1345,6 @@ def parse_listener_def(listener: Any) -> ListenerConfig: configuration. """ -USER_UPDATE_DIRECTORY_DEPRECATION_WARNING = """ -Synapse now uses 'worker_to_update_user_directory' over 'update_user_directory', -you have set 'update_user_directory', and while synapse will work in a backwards -compatible manner, it is suggested to change this value to use -'worker_to_update_user_directory' instead. -""" - def _warn_if_webclient_configured(listeners: Iterable[ListenerConfig]) -> None: for listener in listeners: diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 450799203112..0dfd1035c9f9 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -12,7 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import List, Union +import logging +from typing import Any, List, Union import attr @@ -24,6 +25,16 @@ ) from .server import ListenerConfig, parse_listener_def +logger = logging.Logger(__name__) + +# an object to pass to the "default" parameter, with which we can +# `is` against to be sure we have an undefined config option. +UNDEFINED = object() + +# an object to pass to self.worker_to_update_user_directory if +# update_user_directory was defined, is used to compare with 'is'. +ANY_USER_DIRECTORY_WORKER = object() + _FEDERATION_SENDER_WITH_SEND_FEDERATION_ENABLED_ERROR = """ The send_federation config option must be disabled in the main synapse process before they can be run in a separate worker. @@ -38,6 +49,13 @@ Please add ``start_pushers: false`` to the main config """ +USER_UPDATE_DIRECTORY_DEPRECATION_WARNING = """ +Synapse now uses 'worker_to_update_user_directory' over 'update_user_directory', +you have set 'update_user_directory', and while synapse will work in a backwards +compatible manner, it is suggested to change this value to use +'worker_to_update_user_directory' instead. +""" + def _instance_to_list_converter(obj: Union[str, List[str]]) -> List[str]: """Helper for allowing parsing a string or list of strings to a config @@ -294,6 +312,37 @@ def read_config(self, config, **kwargs): self.worker_name is None and background_tasks_instance == "master" ) or self.worker_name == background_tasks_instance + # Which worker is responsible for updating the user directory, + # None means the main process handles this. + # Eventually this is expected to hold None, a `str`, or ANY_USER_DIRECTORY_WORKER + # Consider this an `Any`, only match positively with '== worker_name', 'is None' or + # 'is ANY_USER_DIRECTORY_WORKER' to determine if the local process should be updating the directory. + self.update_user_directory_on: Any = config.get( + "update_user_directory_on", UNDEFINED + ) + + update_user_directory = config.get("update_user_directory", UNDEFINED) + + # Resolve backwards compat between update_user_directory (UUD) and + # worker_to_update_user_directory (WTUUD): + # - if WTUUD is defined, just use it + # - if UUD and WTUUD are undefined, assume WTUUD is None + # - if UUD is defined (and True), assume WTUUD is None + # - if UUD is defined (and False), set sentinel object so that user_dir + # workers will work normally. + if self.update_user_directory_on is UNDEFINED: + if update_user_directory is UNDEFINED: + self.update_user_directory_on = None + else: + logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) + if update_user_directory: + self.update_user_directory_on = None + else: + self.update_user_directory_on = ANY_USER_DIRECTORY_WORKER + + # Via all branches, this value is defined + assert self.update_user_directory_on is not UNDEFINED + def generate_config_section(self, config_dir_path, server_name, **kwargs): return """\ ## Workers ## @@ -335,6 +384,12 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # #run_background_tasks_on: worker1 + # The worker that is used to run user directory update tasks + # (e.g. users in public rooms, shared rooms between users.). + # + # If not provided, or null, this defaults to the main process. + #update_user_directory_on: null + # A shared secret used by the replication APIs to authenticate HTTP requests # from workers. # diff --git a/synapse/server.py b/synapse/server.py index d4638a96d3be..ae403854528a 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -44,7 +44,7 @@ from synapse.appservice.api import ApplicationServiceApi from synapse.appservice.scheduler import ApplicationServiceScheduler from synapse.config.homeserver import HomeServerConfig -from synapse.config.server import ANY_USER_DIRECTORY_WORKER +from synapse.config.workers import ANY_USER_DIRECTORY_WORKER from synapse.crypto import context_factory from synapse.crypto.context_factory import RegularPolicyForHTTPS from synapse.crypto.keyring import Keyring @@ -839,16 +839,13 @@ def should_update_user_directory(self) -> bool: "Should this process be updating the user directory?" if self.config.worker.worker_app is None: # we're the main process - return self.config.server.worker_to_update_user_directory is None - elif ( - self.config.server.worker_to_update_user_directory - is ANY_USER_DIRECTORY_WORKER - ): + return self.config.worker.update_user_directory_on is None + elif self.config.worker.update_user_directory_on is ANY_USER_DIRECTORY_WORKER: # A special backwards-compatible case for if update_user_directory is False return self.config.worker.worker_app == "synapse.app.user_dir" else: return ( - self.config.server.worker_to_update_user_directory + self.config.worker.update_user_directory_on == self.config.worker.worker_name ) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 0bbb6acd1e83..4b1419440753 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -56,7 +56,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() - config["worker_to_update_user_directory"] = None + config["update_user_directory_on"] = None self.appservice = ApplicationService( token="i_am_an_app_service", @@ -1014,7 +1014,7 @@ class TestUserDirSearchDisabled(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() - config["worker_to_update_user_directory"] = None + config["update_user_directory_on"] = None hs = self.setup_test_homeserver(config=config) self.config = hs.config diff --git a/tests/rest/client/test_shared_rooms.py b/tests/rest/client/test_shared_rooms.py index caa3554c6f88..841ad01adce7 100644 --- a/tests/rest/client/test_shared_rooms.py +++ b/tests/rest/client/test_shared_rooms.py @@ -32,7 +32,7 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() - config["worker_to_update_user_directory"] = None + config["update_user_directory_on"] = None return self.setup_test_homeserver(config=config) def prepare(self, reactor, clock, hs): diff --git a/tests/utils.py b/tests/utils.py index 6c28a88e6df5..026da6a5ca46 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -169,7 +169,7 @@ def default_config(name, parse=False): "default_room_version": DEFAULT_ROOM_VERSION, # disable user directory updates, because they get done in the # background, which upsets the test runner. - "worker_to_update_user_directory": "B. F. Bugleberry", + "update_user_directory_on": "", "caches": {"global_factor": 1}, "listeners": [{"port": 0, "type": "http"}], } From ec9f0357800365a08bf195e0c1562ed867d9dc7b Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Thu, 9 Dec 2021 16:36:33 +0100 Subject: [PATCH 18/25] i sort, you sort, we sort --- synapse/config/workers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 35a27acd4185..5f80fabdae4d 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -13,8 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -import logging import argparse +import logging from typing import Any, List, Union import attr From 99b6f602b8f43cd92b036396e888ae454a35d825 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sat, 15 Jan 2022 15:48:30 +0100 Subject: [PATCH 19/25] apply review feedback --- docs/sample_config.yaml | 3 +- synapse/app/admin_cmd.py | 2 +- synapse/app/generic_worker.py | 13 ++---- synapse/config/workers.py | 67 +++++++++++++----------------- synapse/handlers/user_directory.py | 2 +- synapse/server.py | 15 ------- 6 files changed, 38 insertions(+), 64 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index a065b16fc9dc..87b8f2984f09 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -2661,7 +2661,8 @@ opentracing: # (e.g. users in public rooms, shared rooms between users.). # # If not provided, or null, this defaults to the main process. -#update_user_directory_on: null +# +#update_user_directory_on: "sync1" # 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 b4a190b13f1a..60c0e37599f5 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -211,7 +211,7 @@ def start(config_options: List[str]) -> None: config.logging.no_redirect_stdio = True # Explicitly disable background processes - config.worker.update_user_directory_on = "" + config.worker.update_user_directory = False config.worker.run_background_tasks = False config.worker.start_pushers = False config.worker.pusher_shard_config.instances = [] diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index e7a8ce75d218..7f98f0b5c30b 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -40,7 +40,7 @@ from synapse.config._base import ConfigError from synapse.config.homeserver import HomeServerConfig from synapse.config.logger import setup_logging -from synapse.config.workers import ANY_USER_DIRECTORY_WORKER, ListenerConfig +from synapse.config.workers import ListenerConfig from synapse.federation.transport.server import TransportLayerServer from synapse.http.server import JsonResource, OptionsResource from synapse.http.servlet import RestServlet, parse_json_object_from_request @@ -458,16 +458,11 @@ def start(config_options: List[str]) -> None: config.appservice.notify_appservices = False if config.worker.worker_app == "synapse.app.user_dir": - if ( - config.worker.worker_name != config.worker.update_user_directory_on - and config.worker.update_user_directory_on is not ANY_USER_DIRECTORY_WORKER - ): + if not config.worker.update_user_directory: sys.stderr.write( - "\nThe update_user_directory_on config variable must point to this worker's name" - "\nto give this worker exclusive access to run it. (It is currently pointed at " - f"{config.worker.update_user_directory_on!r})" + "\nThe user directory worker is not allowed to perform user directory updates per the config." "\nPlease add or edit " - f'``update_user_directory_on: "{config.worker.worker_name}"`` in the main config\n' + f'``update_user_directory_on: "{config.worker.worker_name}"`` in the config\n' ) sys.exit(1) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 5f80fabdae4d..6538f3c1aa91 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -15,7 +15,7 @@ import argparse import logging -from typing import Any, List, Union +from typing import List, Union import attr @@ -29,14 +29,6 @@ logger = logging.Logger(__name__) -# an object to pass to the "default" parameter, with which we can -# `is` against to be sure we have an undefined config option. -UNDEFINED = object() - -# an object to pass to self.worker_to_update_user_directory if -# update_user_directory was defined, is used to compare with 'is'. -ANY_USER_DIRECTORY_WORKER = object() - _FEDERATION_SENDER_WITH_SEND_FEDERATION_ENABLED_ERROR = """ The send_federation config option must be disabled in the main synapse process before they can be run in a separate worker. @@ -314,36 +306,36 @@ def read_config(self, config, **kwargs): self.worker_name is None and background_tasks_instance == "master" ) or self.worker_name == background_tasks_instance - # Which worker is responsible for updating the user directory, - # None means the main process handles this. - # Eventually this is expected to hold None, a `str`, or ANY_USER_DIRECTORY_WORKER - # Consider this an `Any`, only match positively with '== worker_name', 'is None' or - # 'is ANY_USER_DIRECTORY_WORKER' to determine if the local process should be updating the directory. - self.update_user_directory_on: Any = config.get( - "update_user_directory_on", UNDEFINED - ) + # update_user_directory_on controls which worker is responsible for updating the user directory. + # None means that the main process should handle this, so does the absence of the config key. + # + # Note that due to backwards compatibility, we must also test for update_user_directory, and apply it if + # update_user_directory_on is not defined at the same time. + self.update_user_directory: bool + + uud_result: bool + is_master_process = self.worker_name is None - update_user_directory = config.get("update_user_directory", UNDEFINED) - - # Resolve backwards compat between update_user_directory (UUD) and - # worker_to_update_user_directory (WTUUD): - # - if WTUUD is defined, just use it - # - if UUD and WTUUD are undefined, assume WTUUD is None - # - if UUD is defined (and True), assume WTUUD is None - # - if UUD is defined (and False), set sentinel object so that user_dir - # workers will work normally. - if self.update_user_directory_on is UNDEFINED: - if update_user_directory is UNDEFINED: - self.update_user_directory_on = None + if "update_user_directory_on" in config: + update_user_directory_on = config["update_user_directory_on"] + + if update_user_directory_on is None: + uud_result = is_master_process + else: + uud_result = self.worker_name == update_user_directory_on + + else: + if "update_user_directory" in config: + # Backwards compatibility case + uud_result = self.worker_app == "synapse.app.user_dir" else: - logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - if update_user_directory: - self.update_user_directory_on = None - else: - self.update_user_directory_on = ANY_USER_DIRECTORY_WORKER + # Both values are undefined, assume None for update_user_directory_on + uud_result = is_master_process - # Via all branches, this value is defined - assert self.update_user_directory_on is not UNDEFINED + if "update_user_directory" in config: + logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) + + self.update_user_directory = uud_result def generate_config_section(self, config_dir_path, server_name, **kwargs): return """\ @@ -390,7 +382,8 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # (e.g. users in public rooms, shared rooms between users.). # # If not provided, or null, this defaults to the main process. - #update_user_directory_on: null + # + #update_user_directory_on: "sync1" # 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 e2632859b54a..d9a701a86fa8 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -60,7 +60,7 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self.notifier = hs.get_notifier() self.is_mine_id = hs.is_mine_id - self.update_user_directory = hs.should_update_user_directory() + self.update_user_directory = hs.config.worker.update_user_directory self.search_all_users = hs.config.userdirectory.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/server.py b/synapse/server.py index ed32b5929a1a..185e40e4da0f 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -44,7 +44,6 @@ from synapse.appservice.api import ApplicationServiceApi from synapse.appservice.scheduler import ApplicationServiceScheduler from synapse.config.homeserver import HomeServerConfig -from synapse.config.workers import ANY_USER_DIRECTORY_WORKER from synapse.crypto import context_factory from synapse.crypto.context_factory import RegularPolicyForHTTPS from synapse.crypto.keyring import Keyring @@ -840,20 +839,6 @@ def should_send_federation(self) -> bool: "Should this server be sending federation traffic directly?" return self.config.worker.send_federation - def should_update_user_directory(self) -> bool: - "Should this process be updating the user directory?" - - if self.config.worker.worker_app is None: # we're the main process - return self.config.worker.update_user_directory_on is None - elif self.config.worker.update_user_directory_on is ANY_USER_DIRECTORY_WORKER: - # A special backwards-compatible case for if update_user_directory is False - return self.config.worker.worker_app == "synapse.app.user_dir" - else: - return ( - self.config.worker.update_user_directory_on - == self.config.worker.worker_name - ) - @cache_in_self def get_request_ratelimiter(self) -> RequestRatelimiter: return RequestRatelimiter( From db78c2d0a2e66f36565be66bead9019ecfb0459f Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sat, 15 Jan 2022 16:14:19 +0100 Subject: [PATCH 20/25] change when deprecation message shows up --- synapse/config/workers.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 6538f3c1aa91..b166b9d7924a 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -44,10 +44,10 @@ """ USER_UPDATE_DIRECTORY_DEPRECATION_WARNING = """ -Synapse now uses 'worker_to_update_user_directory' over 'update_user_directory', +Synapse now uses 'update_user_directory_on' over 'update_user_directory', you have set 'update_user_directory', and while synapse will work in a backwards compatible manner, it is suggested to change this value to use -'worker_to_update_user_directory' instead. +'update_user_directory_on' instead. """ @@ -327,14 +327,12 @@ def read_config(self, config, **kwargs): else: if "update_user_directory" in config: # Backwards compatibility case + logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) uud_result = self.worker_app == "synapse.app.user_dir" else: # Both values are undefined, assume None for update_user_directory_on uud_result = is_master_process - if "update_user_directory" in config: - logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - self.update_user_directory = uud_result def generate_config_section(self, config_dir_path, server_name, **kwargs): From f54146d570b448e2633b2d5809abfe50c07ed838 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sat, 15 Jan 2022 16:23:13 +0100 Subject: [PATCH 21/25] fix backwards compatibility case --- synapse/config/workers.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index b166b9d7924a..ca8f8084c88c 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -15,7 +15,7 @@ import argparse import logging -from typing import List, Union +from typing import List, Union, Optional import attr @@ -317,7 +317,7 @@ def read_config(self, config, **kwargs): is_master_process = self.worker_name is None if "update_user_directory_on" in config: - update_user_directory_on = config["update_user_directory_on"] + update_user_directory_on: Optional[str] = config["update_user_directory_on"] if update_user_directory_on is None: uud_result = is_master_process @@ -328,7 +328,15 @@ def read_config(self, config, **kwargs): if "update_user_directory" in config: # Backwards compatibility case logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - uud_result = self.worker_app == "synapse.app.user_dir" + + update_user_directory: bool = config['update_user_directory'] + + if update_user_directory: + # Main process should update + uud_result = is_master_process + else: + # user directory worker should update + uud_result = self.worker_app == "synapse.app.user_dir" else: # Both values are undefined, assume None for update_user_directory_on uud_result = is_master_process From cec33109cec598c0acca6431de7dddf507a1c594 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sat, 15 Jan 2022 16:26:14 +0100 Subject: [PATCH 22/25] mighty linter --- synapse/config/workers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index ca8f8084c88c..a64bad64e231 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -15,7 +15,7 @@ import argparse import logging -from typing import List, Union, Optional +from typing import List, Optional, Union import attr @@ -329,7 +329,7 @@ def read_config(self, config, **kwargs): # Backwards compatibility case logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - update_user_directory: bool = config['update_user_directory'] + update_user_directory: bool = config["update_user_directory"] if update_user_directory: # Main process should update From 1e9b096f884a4c1b7722e2c6467b449b3862932d Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 30 Jan 2022 12:32:39 +0100 Subject: [PATCH 23/25] Apply review feedback Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/app/generic_worker.py | 12 +++--- synapse/config/workers.py | 55 ++++++++++----------------- tests/handlers/test_user_directory.py | 1 + tests/utils.py | 2 +- 4 files changed, 28 insertions(+), 42 deletions(-) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 7f98f0b5c30b..36d6f5a6fef8 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -126,6 +126,11 @@ logger = logging.getLogger("synapse.app.generic_worker") +DIRECTORY_UPDATE_WARNING = """ +The user directory worker is not allowed to perform user directory updates per the config. +Please add or edit ``update_user_directory_on: "{0}"`` in the config' +""" + class KeyUploadServlet(RestServlet): """An implementation of the `KeyUploadServlet` that responds to read only @@ -459,12 +464,7 @@ def start(config_options: List[str]) -> None: if config.worker.worker_app == "synapse.app.user_dir": if not config.worker.update_user_directory: - sys.stderr.write( - "\nThe user directory worker is not allowed to perform user directory updates per the config." - "\nPlease add or edit " - f'``update_user_directory_on: "{config.worker.worker_name}"`` in the config\n' - ) - sys.exit(1) + logger.warning(DIRECTORY_UPDATE_WARNING.format(config.worker.worker_name)) synapse.events.USE_FROZEN_DICTS = config.server.use_frozen_dicts synapse.util.caches.TRACK_MEMORY_USAGE = config.caches.track_memory_usage diff --git a/synapse/config/workers.py b/synapse/config/workers.py index a64bad64e231..4b08ae8f2f54 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -15,7 +15,7 @@ import argparse import logging -from typing import List, Optional, Union +from typing import List, Union import attr @@ -44,10 +44,9 @@ """ USER_UPDATE_DIRECTORY_DEPRECATION_WARNING = """ -Synapse now uses 'update_user_directory_on' over 'update_user_directory', -you have set 'update_user_directory', and while synapse will work in a backwards -compatible manner, it is suggested to change this value to use -'update_user_directory_on' instead. +The 'update_user_directory' configuration setting is now deprecated, +and replaced with 'update_user_directory_on'. See the sample configuration +file for details of 'update_user_directory_on'. """ @@ -302,46 +301,32 @@ def read_config(self, config, **kwargs): # No effort is made to ensure only a single instance of these tasks is # running. background_tasks_instance = config.get("run_background_tasks_on") or "master" - self.run_background_tasks = ( - self.worker_name is None and background_tasks_instance == "master" - ) or self.worker_name == background_tasks_instance + self.run_background_tasks = self.instance_name == background_tasks_instance # update_user_directory_on controls which worker is responsible for updating the user directory. - # None means that the main process should handle this, so does the absence of the config key. + # None means that the main process should handle this, as does the absence of the config key. # # Note that due to backwards compatibility, we must also test for update_user_directory, and apply it if # update_user_directory_on is not defined at the same time. - self.update_user_directory: bool - uud_result: bool - is_master_process = self.worker_name is None + if "update_user_directory" in config: + # Backwards compatibility case + logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - if "update_user_directory_on" in config: - update_user_directory_on: Optional[str] = config["update_user_directory_on"] - - if update_user_directory_on is None: - uud_result = is_master_process + if config["update_user_directory"]: + # Main process should update + update_user_directory_on = "master" else: - uud_result = self.worker_name == update_user_directory_on - + # user directory worker should update + update_user_directory_on = "synapse.app.user_dir" else: - if "update_user_directory" in config: - # Backwards compatibility case - logger.warning(USER_UPDATE_DIRECTORY_DEPRECATION_WARNING) - - update_user_directory: bool = config["update_user_directory"] - - if update_user_directory: - # Main process should update - uud_result = is_master_process - else: - # user directory worker should update - uud_result = self.worker_app == "synapse.app.user_dir" - else: - # Both values are undefined, assume None for update_user_directory_on - uud_result = is_master_process + update_user_directory_on = ( + config.get("update_user_directory_on") or "master" + ) - self.update_user_directory = uud_result + self.update_user_directory: bool = ( + self.instance_name == update_user_directory_on + ) def generate_config_section(self, config_dir_path, server_name, **kwargs): return """\ diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 4b1419440753..20e13f86ecd1 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -56,6 +56,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() + # Always process user directory updates on this main homeserver process. config["update_user_directory_on"] = None self.appservice = ApplicationService( diff --git a/tests/utils.py b/tests/utils.py index 026da6a5ca46..125546a91c9e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -169,7 +169,7 @@ def default_config(name, parse=False): "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_on": "", + "update_user_directory_on": "(non-existent worker)", "caches": {"global_factor": 1}, "listeners": [{"port": 0, "type": "http"}], } From 4eb94165a35be179a82881d170540bd1292cdaab Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 30 Jan 2022 13:02:14 +0100 Subject: [PATCH 24/25] fix regression --- synapse/config/workers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 4b08ae8f2f54..ebef7f3a4cf1 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -318,7 +318,11 @@ def read_config(self, config, **kwargs): update_user_directory_on = "master" else: # user directory worker should update - update_user_directory_on = "synapse.app.user_dir" + update_user_directory_on = ( + self.instance_name + if self.worker_app == "synapse.app.user_dir" + else "" + ) else: update_user_directory_on = ( config.get("update_user_directory_on") or "master" From dca6e222288dc1f5298fec38f803cb9a3ef2210a Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Tue, 1 Feb 2022 16:36:28 +0100 Subject: [PATCH 25/25] apply review feedback --- synapse/app/generic_worker.py | 4 ++-- synapse/config/workers.py | 4 +--- tests/handlers/test_user_directory.py | 3 ++- tests/rest/client/test_shared_rooms.py | 1 + 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 36d6f5a6fef8..71419d34232d 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -127,7 +127,7 @@ logger = logging.getLogger("synapse.app.generic_worker") DIRECTORY_UPDATE_WARNING = """ -The user directory worker is not allowed to perform user directory updates per the config. +The user directory worker is not configured to perform user directory updates per the config. Please add or edit ``update_user_directory_on: "{0}"`` in the config' """ @@ -464,7 +464,7 @@ def start(config_options: List[str]) -> None: if config.worker.worker_app == "synapse.app.user_dir": if not config.worker.update_user_directory: - logger.warning(DIRECTORY_UPDATE_WARNING.format(config.worker.worker_name)) + sys.stderr.write(DIRECTORY_UPDATE_WARNING.format(config.worker.worker_name)) synapse.events.USE_FROZEN_DICTS = config.server.use_frozen_dicts synapse.util.caches.TRACK_MEMORY_USAGE = config.caches.track_memory_usage diff --git a/synapse/config/workers.py b/synapse/config/workers.py index ebef7f3a4cf1..6188454c9ae6 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -328,9 +328,7 @@ def read_config(self, config, **kwargs): config.get("update_user_directory_on") or "master" ) - self.update_user_directory: bool = ( - self.instance_name == update_user_directory_on - ) + self.update_user_directory = self.instance_name == update_user_directory_on def generate_config_section(self, config_dir_path, server_name, **kwargs): return """\ diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 20e13f86ecd1..8f2adf765174 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -56,7 +56,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() - # Always process user directory updates on this main homeserver process. + # Enable user directory updates, as the test homeserver must always process them for tests. config["update_user_directory_on"] = None self.appservice = ApplicationService( @@ -1015,6 +1015,7 @@ class TestUserDirSearchDisabled(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() + # Enable user directory updates, as the test homeserver must always process them for tests. config["update_user_directory_on"] = None hs = self.setup_test_homeserver(config=config) diff --git a/tests/rest/client/test_shared_rooms.py b/tests/rest/client/test_shared_rooms.py index 841ad01adce7..c3fb5d86c781 100644 --- a/tests/rest/client/test_shared_rooms.py +++ b/tests/rest/client/test_shared_rooms.py @@ -32,6 +32,7 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() + # Enable user directory updates, as the test homeserver must always process them for tests. config["update_user_directory_on"] = None return self.setup_test_homeserver(config=config)