Add the `update_user_directory_from_worker` configuration option (superseding `update_user_directory`) to allow a generic worker to be designated as the worker to update the user directory. (#12654)

Co-authored-by: Shay <hillerys@element.io>

9 files changed, 76 insertions, 34 deletions

diff --git a/changelog.d/12654.feature b/changelog.d/12654.feature
new file mode 100644
index 0000000000..fd0a4bb0da
--- /dev/null
+++ b/changelog.d/12654.feature
@@ -0,0 +1 @@
+Add the `update_user_directory_from_worker` configuration option (superseding `update_user_directory`) to allow a generic worker to be designated as the worker to update the user directory.
\ No newline at end of file
diff --git a/docs/upgrade.md b/docs/upgrade.md
index 18c33a4198..fa4b3ef590 100644
--- a/docs/upgrade.md
+++ b/docs/upgrade.md
@@ -101,29 +101,36 @@ To re-enable this functionality, set the
 homeserver config option to `true`.
 
 
-## Deprecation of the `synapse.app.appservice` worker application type
+## Deprecation of the `synapse.app.appservice` and `synapse.app.user_dir` worker application types
 
 The `synapse.app.appservice` worker application type allowed you to configure a
 single worker to use to notify application services of new events, as long
 as this functionality was disabled on the main process with `notify_appservices: False`.
+Further, the `synapse.app.user_dir` worker application type allowed you to configure
+a single worker to be responsible for updating the user directory, as long as this
+was disabled on the main process with `update_user_directory: False`.
 
 To unify Synapse's worker types, the `synapse.app.appservice` worker application
 type and the `notify_appservices` configuration option have been deprecated.
+The `synapse.app.user_dir` worker application type and `update_user_directory`
+configuration option have also been deprecated.
 
-To get the same functionality, it's now recommended that the `synapse.app.generic_worker`
-worker application type is used and that the `notify_appservices_from_worker` option
-is set to the name of a worker.
+To get the same functionality as was provided by the deprecated options, it's now recommended that the `synapse.app.generic_worker`
+worker application type is used and that the `notify_appservices_from_worker` and/or
+`update_user_directory_from_worker` options are set to the name of a worker.
 
-For the time being, `notify_appservices_from_worker` can be used alongside
-`synapse.app.appservice` and `notify_appservices` to make it easier to transition
-between the two configurations, however please note that:
+For the time being, the old options can be used alongside the new options to make
+it easier to transition between the two configurations, however please note that:
 
 - the options must not contradict each other (otherwise Synapse won't start); and
-- the `notify_appservices` option will be removed in a future release of Synapse.
+- the `notify_appservices` and `update_user_directory` options will be removed in a future release of Synapse.
 
-Please see [the relevant section of the worker documentation][v1_59_notify_ases_from] for more information.
+Please see the [*Notifying Application Services*][v1_59_notify_ases_from] and
+[*Updating the User Directory*][v1_59_update_user_dir] sections of the worker
+documentation for more information.
 
 [v1_59_notify_ases_from]: workers.md#notifying-application-services
+[v1_59_update_user_dir]: workers.md#updating-the-user-directory
 
 
 # Upgrading to v1.58.0
diff --git a/docs/workers.md b/docs/workers.md
index 1d049b6c4f..553792d238 100644
--- a/docs/workers.md
+++ b/docs/workers.md
@@ -426,7 +426,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 `update_user_directory_from_worker` and
 `media_instance_running_background_jobs` settings.
 
 An example for a dedicated background worker instance:
@@ -435,9 +435,26 @@ An example for a dedicated background worker instance:
 {{#include systemd-with-workers/workers/background_worker.yaml}}
 ```
 
+#### Updating the User Directory
+
+You can designate one generic worker to update the user directory.
+
+Specify its name in the shared configuration as follows:
+
+```yaml
+update_user_directory_from_worker: worker_name
+```
+
+This work cannot be load-balanced; please ensure the main process is restarted
+after setting this option in the shared configuration!
+
+This style of configuration supersedes the legacy `synapse.app.user_dir`
+worker application type.
+
+
 #### Notifying Application Services
 
-You can designate one worker to send output traffic to Application Services.
+You can designate one generic worker to send output traffic to Application Services.
 
 Specify its name in the shared configuration as follows:
 
@@ -470,7 +487,7 @@ pusher_instances:
 
 ### `synapse.app.appservice`
 
-**Deprecated as of Synapse v1.58.** [Use `synapse.app.generic_worker` with the
+**Deprecated as of Synapse v1.59.** [Use `synapse.app.generic_worker` with the
 `notify_appservices_from_worker` option instead.](#notifying-application-services)
 
 Handles sending output traffic to Application Services. Doesn't handle any
@@ -540,6 +557,9 @@ Note that if a reverse proxy is used , then `/_matrix/media/` must be routed for
 
 ### `synapse.app.user_dir`
 
+**Deprecated as of Synapse v1.59.** [Use `synapse.app.generic_worker` with the
+`update_user_directory_from_worker` option instead.](#updating-the-user-directory)
+
 Handles searches in the user directory. It can handle REST endpoints matching
 the following regular expressions:
 
diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py
index 2b0d92cbae..2a4c2e59cd 100644
--- a/synapse/app/admin_cmd.py
+++ b/synapse/app/admin_cmd.py
@@ -210,7 +210,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.worker.should_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 07dddc0b13..2a9480a5c1 100644
--- a/synapse/app/generic_worker.py
+++ b/synapse/app/generic_worker.py
@@ -441,22 +441,6 @@ def start(config_options: List[str]) -> None:
         "synapse.app.user_dir",
     )
 
-    if config.worker.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.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 1e709c7cf5..005a3ee48c 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -319,10 +319,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 a9dbcc6d3d..e1569b3c14 100644
--- a/synapse/config/workers.py
+++ b/synapse/config/workers.py
@@ -311,6 +311,13 @@ class WorkerConfig(Config):
             new_option_name="notify_appservices_from_worker",
         )
 
+        self.should_update_user_directory = self._should_this_worker_perform_duty(
+            config,
+            legacy_master_option_name="update_user_directory",
+            legacy_worker_app_name="synapse.app.user_dir",
+            new_option_name="update_user_directory_from_worker",
+        )
+
     def _should_this_worker_perform_duty(
         self,
         config: Dict[str, Any],
diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py
index 048fd4bb82..74f7fdfe6c 100644
--- a/synapse/handlers/user_directory.py
+++ b/synapse/handlers/user_directory.py
@@ -60,7 +60,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.server.update_user_directory
+        self.update_user_directory = hs.config.worker.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/tests/config/test_workers.py b/tests/config/test_workers.py
index da81bb9655..ef6294ecb2 100644
--- a/tests/config/test_workers.py
+++ b/tests/config/test_workers.py
@@ -286,3 +286,30 @@ class WorkerDutyConfigTestCase(TestCase):
                 "notify_appservices_from_worker",
             )
         )
+
+    def test_worker_duty_configs(self) -> None:
+        """
+        Additional tests for the worker duties
+        """
+
+        worker1_config = self._make_worker_config(
+            worker_app="synapse.app.generic_worker",
+            worker_name="worker1",
+            extras={
+                "notify_appservices_from_worker": "worker2",
+                "update_user_directory_from_worker": "worker1",
+            },
+        )
+        self.assertFalse(worker1_config.should_notify_appservices)
+        self.assertTrue(worker1_config.should_update_user_directory)
+
+        worker2_config = self._make_worker_config(
+            worker_app="synapse.app.generic_worker",
+            worker_name="worker2",
+            extras={
+                "notify_appservices_from_worker": "worker2",
+                "update_user_directory_from_worker": "worker1",
+            },
+        )
+        self.assertTrue(worker2_config.should_notify_appservices)
+        self.assertFalse(worker2_config.should_update_user_directory)