Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Introduce update_user_directory_on #11450

Closed
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9a3d3b6
shift to worker_to_update_user_directory
ShadowJonathan Nov 29, 2021
5f6bce7
linter
ShadowJonathan Nov 29, 2021
f24f151
news
ShadowJonathan Nov 29, 2021
0f05a6d
update nonsensical name in default_config() to be more inspired
ShadowJonathan Nov 29, 2021
300b51e
add updated sample config
ShadowJonathan Nov 29, 2021
28e255c
update sample config the sequel
ShadowJonathan Nov 29, 2021
47134d1
fix mypy issue
ShadowJonathan Nov 29, 2021
4d518bd
fix sytest
ShadowJonathan Nov 29, 2021
b67fe88
debug sytest failures
ShadowJonathan Nov 29, 2021
7ede21e
debug sytest failures mk 2
ShadowJonathan Nov 29, 2021
434d108
add backwards compatibility
ShadowJonathan Dec 6, 2021
f2490b3
update config with false behaviour
ShadowJonathan Dec 6, 2021
4593e15
whoops
ShadowJonathan Dec 6, 2021
db6625e
add special case for user directory workers
ShadowJonathan Dec 7, 2021
6f6b83b
the lint, it sees all, knows all
ShadowJonathan Dec 7, 2021
7808e86
apply review feedback
ShadowJonathan Dec 8, 2021
2d2c904
update name, move to workers.py
ShadowJonathan Dec 9, 2021
171020d
Merge branch 'develop' into fix-update-user-dir
ShadowJonathan Dec 9, 2021
ec9f035
i sort, you sort, we sort
ShadowJonathan Dec 9, 2021
99b6f60
apply review feedback
ShadowJonathan Jan 15, 2022
db78c2d
change when deprecation message shows up
ShadowJonathan Jan 15, 2022
f54146d
fix backwards compatibility case
ShadowJonathan Jan 15, 2022
cec3310
mighty linter
ShadowJonathan Jan 15, 2022
1e9b096
Apply review feedback
ShadowJonathan Jan 30, 2022
4eb9416
fix regression
ShadowJonathan Jan 30, 2022
dca6e22
apply review feedback
ShadowJonathan Feb 1, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11450.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Introduce `update_user_directory_on` config variable.
2 changes: 1 addition & 1 deletion docker/configure_workers_and_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"endpoint_patterns": [
"^/_matrix/client/(api/v1|r0|v3|unstable)/user_directory/search$"
],
"shared_extra_conf": {"update_user_directory": False},
"shared_extra_conf": {"update_user_directory_on": "user_dir1"},
"worker_extra_conf": "",
},
"media_repository": {
Expand Down
6 changes: 6 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2657,6 +2657,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
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved

# A shared secret used by the replication APIs to authenticate HTTP requests
# from workers.
#
Expand Down
9 changes: 5 additions & 4 deletions docs/workers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `update_user_directory_on` and
`media_instance_running_background_jobs` settings.

### `synapse.app.pusher`
Expand Down Expand Up @@ -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 `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.

### `synapse.app.frontend_proxy`

Expand Down
2 changes: 1 addition & 1 deletion synapse/app/admin_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.worker.update_user_directory_on = ""
config.worker.run_background_tasks = False
config.worker.start_pushers = False
config.worker.pusher_shard_config.instances = []
Expand Down
22 changes: 10 additions & 12 deletions synapse/app/generic_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.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
Expand Down Expand Up @@ -458,21 +458,19 @@ 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.worker.update_user_directory_on
and config.worker.update_user_directory_on is not ANY_USER_DIRECTORY_WORKER
):
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 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})"
"\nPlease add or edit "
f'``update_user_directory_on: "{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

Expand Down
4 changes: 0 additions & 4 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,6 @@ 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)
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved

# 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
Expand Down
57 changes: 56 additions & 1 deletion synapse/config/workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
# limitations under the License.

import argparse
from typing import List, Union
import logging
from typing import Any, List, Union

import attr

Expand All @@ -26,6 +27,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()
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved

# 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()
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved

_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.
Expand All @@ -40,6 +51,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
Expand Down Expand Up @@ -296,6 +314,37 @@ def read_config(self, config, **kwargs):
self.worker_name is None and background_tasks_instance == "master"
) or self.worker_name == background_tasks_instance
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved

# 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 ##
Expand Down Expand Up @@ -337,6 +386,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.
#
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions synapse/rest/client/shared_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

squahtx marked this conversation as resolved.
Show resolved Hide resolved
UserID.from_string(user_id)

requester = await self.auth.get_user_by_req(request)
Expand Down
15 changes: 15 additions & 0 deletions synapse/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.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
Expand Down Expand Up @@ -839,6 +840,20 @@ 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(
Expand Down
4 changes: 2 additions & 2 deletions tests/handlers/test_user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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["update_user_directory_on"] = None
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved

self.appservice = ApplicationService(
token="i_am_an_app_service",
Expand Down Expand Up @@ -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["update_user_directory_on"] = None
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved
hs = self.setup_test_homeserver(config=config)

self.config = hs.config
Expand Down
2 changes: 1 addition & 1 deletion tests/rest/client/test_shared_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class UserSharedRoomsTest(unittest.HomeserverTestCase):

def make_homeserver(self, reactor, clock):
config = self.default_config()
config["update_user_directory"] = True
config["update_user_directory_on"] = None
return self.setup_test_homeserver(config=config)

def prepare(self, reactor, clock, hs):
Expand Down
2 changes: 1 addition & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
"update_user_directory_on": "",
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved
"caches": {"global_factor": 1},
"listeners": [{"port": 0, "type": "http"}],
}
Expand Down