diff --git a/changelog.d/18737.removal b/changelog.d/18737.removal new file mode 100644 index 000000000..7b45600c3 --- /dev/null +++ b/changelog.d/18737.removal @@ -0,0 +1 @@ +Deprecate `run_as_background_process` exported as part of the module API interface in favor of `ModuleApi.run_as_background_process`. See the relevant section in the upgrade notes for more information. diff --git a/docs/upgrade.md b/docs/upgrade.md index 9decc4372..f470e4ba2 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -119,6 +119,35 @@ stacking them up. You can monitor the currently running background updates with # Upgrading to v1.136.0 +## Deprecate `run_as_background_process` exported as part of the module API interface in favor of `ModuleApi.run_as_background_process` + +The `run_as_background_process` function is now a method of the `ModuleApi` class. If +you were using the function directly from the module API, it will continue to work fine +but the background process metrics will not include an accurate `server_name` label. +This kind of metric labeling isn't relevant for many use cases and is used to +differentiate Synapse instances running in the same Python process (relevant to Synapse +Pro: Small Hosts). We recommend updating your usage to use the new +`ModuleApi.run_as_background_process` method to stay on top of future changes. + +
+Example run_as_background_process upgrade + +Before: +```python +class MyModule: + def __init__(self, module_api: ModuleApi) -> None: + run_as_background_process(__name__ + ":setup_database", self.setup_database) +``` + +After: +```python +class MyModule: + def __init__(self, module_api: ModuleApi) -> None: + module_api.run_as_background_process(__name__ + ":setup_database", self.setup_database) +``` + +
+ ## Metric labels have changed on `synapse_federation_last_received_pdu_time` and `synapse_federation_last_sent_pdu_time` Previously, the `synapse_federation_last_received_pdu_time` and @@ -136,6 +165,7 @@ this change but you will need to manually update your own existing Grafana dashb using these metrics. + # Upgrading to v1.135.0 ## `on_user_registration` module API callback may now run on any worker diff --git a/synapse/events/auto_accept_invites.py b/synapse/events/auto_accept_invites.py index 300ecfdf7..6873ee9d3 100644 --- a/synapse/events/auto_accept_invites.py +++ b/synapse/events/auto_accept_invites.py @@ -114,7 +114,6 @@ class InviteAutoAccepter: # that occurs when responding to invites over federation (see https://github.com/matrix-org/synapse-auto-accept-invite/issues/12) run_as_background_process( "retry_make_join", - self.server_name, self._retry_make_join, event.state_key, event.state_key, diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index d25217539..7657353e5 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -23,6 +23,7 @@ import logging from typing import ( TYPE_CHECKING, Any, + Awaitable, Callable, Collection, Dict, @@ -80,7 +81,9 @@ from synapse.logging.context import ( make_deferred_yieldable, run_in_background, ) -from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.metrics.background_process_metrics import ( + run_as_background_process as _run_as_background_process, +) from synapse.module_api.callbacks.account_validity_callbacks import ( IS_USER_EXPIRED_CALLBACK, ON_LEGACY_ADMIN_REQUEST, @@ -158,6 +161,9 @@ from synapse.util.caches.descriptors import CachedFunction, cached as _cached from synapse.util.frozenutils import freeze if TYPE_CHECKING: + # Old versions don't have `LiteralString` + from typing_extensions import LiteralString + from synapse.app.generic_worker import GenericWorkerStore from synapse.server import HomeServer @@ -216,6 +222,65 @@ class UserIpAndAgent: last_seen: int +def run_as_background_process( + desc: "LiteralString", + func: Callable[..., Awaitable[Optional[T]]], + *args: Any, + bg_start_span: bool = True, + **kwargs: Any, +) -> "defer.Deferred[Optional[T]]": + """ + XXX: Deprecated: use `ModuleApi.run_as_background_process` instead. + + Run the given function in its own logcontext, with resource metrics + + This should be used to wrap processes which are fired off to run in the + background, instead of being associated with a particular request. + + It returns a Deferred which completes when the function completes, but it doesn't + follow the synapse logcontext rules, which makes it appropriate for passing to + clock.looping_call and friends (or for firing-and-forgetting in the middle of a + normal synapse async function). + + Args: + desc: a description for this background process type + server_name: The homeserver name that this background process is being run for + (this should be `hs.hostname`). + func: a function, which may return a Deferred or a coroutine + bg_start_span: Whether to start an opentracing span. Defaults to True. + Should only be disabled for processes that will not log to or tag + a span. + args: positional args for func + kwargs: keyword args for func + + Returns: + Deferred which returns the result of func, or `None` if func raises. + Note that the returned Deferred does not follow the synapse logcontext + rules. + """ + + logger.warning( + "Using deprecated `run_as_background_process` that's exported from the Module API. " + "Prefer `ModuleApi.run_as_background_process` instead.", + ) + + # Historically, since this function is exported from the module API, we can't just + # change the signature to require a `server_name` argument. Since + # `run_as_background_process` internally in Synapse requires `server_name` now, we + # just have to stub this out with a placeholder value and tell people to use the new + # function instead. + stub_server_name = "synapse_module_running_from_unknown_server" + + return _run_as_background_process( + desc, + stub_server_name, + func, + *args, + bg_start_span=bg_start_span, + **kwargs, + ) + + def cached( *, max_entries: int = 1000, @@ -1323,10 +1388,9 @@ class ModuleApi: if self._hs.config.worker.run_background_tasks or run_on_all_instances: self._clock.looping_call( - run_as_background_process, + self.run_as_background_process, msec, desc, - self.server_name, lambda: maybe_awaitable(f(*args, **kwargs)), ) else: @@ -1382,9 +1446,8 @@ class ModuleApi: return self._clock.call_later( # convert ms to seconds as needed by call_later. msec * 0.001, - run_as_background_process, + self.run_as_background_process, desc, - self.server_name, lambda: maybe_awaitable(f(*args, **kwargs)), ) @@ -1590,6 +1653,44 @@ class ModuleApi: return {key: state_events[event_id] for key, event_id in state_ids.items()} + def run_as_background_process( + self, + desc: "LiteralString", + func: Callable[..., Awaitable[Optional[T]]], + *args: Any, + bg_start_span: bool = True, + **kwargs: Any, + ) -> "defer.Deferred[Optional[T]]": + """Run the given function in its own logcontext, with resource metrics + + This should be used to wrap processes which are fired off to run in the + background, instead of being associated with a particular request. + + It returns a Deferred which completes when the function completes, but it doesn't + follow the synapse logcontext rules, which makes it appropriate for passing to + clock.looping_call and friends (or for firing-and-forgetting in the middle of a + normal synapse async function). + + Args: + desc: a description for this background process type + server_name: The homeserver name that this background process is being run for + (this should be `hs.hostname`). + func: a function, which may return a Deferred or a coroutine + bg_start_span: Whether to start an opentracing span. Defaults to True. + Should only be disabled for processes that will not log to or tag + a span. + args: positional args for func + kwargs: keyword args for func + + Returns: + Deferred which returns the result of func, or `None` if func raises. + Note that the returned Deferred does not follow the synapse logcontext + rules. + """ + return _run_as_background_process( + desc, self.server_name, func, *args, bg_start_span=bg_start_span, **kwargs + ) + async def defer_to_thread( self, f: Callable[P, T],