From 7cb8b4bc67042a39bd1b0e05df46089a2fce1955 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 12 May 2020 03:45:23 +1000 Subject: Allow configuration of Synapse's cache without using synctl or environment variables (#6391) --- synapse/config/cache.py | 164 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 synapse/config/cache.py (limited to 'synapse/config/cache.py') diff --git a/synapse/config/cache.py b/synapse/config/cache.py new file mode 100644 index 0000000000..91036a012e --- /dev/null +++ b/synapse/config/cache.py @@ -0,0 +1,164 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +from typing import Callable, Dict + +from ._base import Config, ConfigError + +# The prefix for all cache factor-related environment variables +_CACHES = {} +_CACHE_PREFIX = "SYNAPSE_CACHE_FACTOR" +_DEFAULT_FACTOR_SIZE = 0.5 +_DEFAULT_EVENT_CACHE_SIZE = "10K" + + +class CacheProperties(object): + def __init__(self): + # The default factor size for all caches + self.default_factor_size = float( + os.environ.get(_CACHE_PREFIX, _DEFAULT_FACTOR_SIZE) + ) + self.resize_all_caches_func = None + + +properties = CacheProperties() + + +def add_resizable_cache(cache_name: str, cache_resize_callback: Callable): + """Register a cache that's size can dynamically change + + Args: + cache_name: A reference to the cache + cache_resize_callback: A callback function that will be ran whenever + the cache needs to be resized + """ + _CACHES[cache_name.lower()] = cache_resize_callback + + # Ensure all loaded caches are sized appropriately + # + # This method should only run once the config has been read, + # as it uses values read from it + if properties.resize_all_caches_func: + properties.resize_all_caches_func() + + +class CacheConfig(Config): + section = "caches" + _environ = os.environ + + @staticmethod + def reset(): + """Resets the caches to their defaults. Used for tests.""" + properties.default_factor_size = float( + os.environ.get(_CACHE_PREFIX, _DEFAULT_FACTOR_SIZE) + ) + properties.resize_all_caches_func = None + _CACHES.clear() + + def generate_config_section(self, **kwargs): + return """\ + ## Caching ## + + # Caching can be configured through the following options. + # + # A cache 'factor' is a multiplier that can be applied to each of + # Synapse's caches in order to increase or decrease the maximum + # number of entries that can be stored. + + # The number of events to cache in memory. Not affected by + # caches.global_factor. + # + #event_cache_size: 10K + + caches: + # Controls the global cache factor, which is the default cache factor + # for all caches if a specific factor for that cache is not otherwise + # set. + # + # This can also be set by the "SYNAPSE_CACHE_FACTOR" environment + # variable. Setting by environment variable takes priority over + # setting through the config file. + # + # Defaults to 0.5, which will half the size of all caches. + # + #global_factor: 1.0 + + # A dictionary of cache name to cache factor for that individual + # cache. Overrides the global cache factor for a given cache. + # + # These can also be set through environment variables comprised + # of "SYNAPSE_CACHE_FACTOR_" + the name of the cache in capital + # letters and underscores. Setting by environment variable + # takes priority over setting through the config file. + # Ex. SYNAPSE_CACHE_FACTOR_GET_USERS_WHO_SHARE_ROOM_WITH_USER=2.0 + # + per_cache_factors: + #get_users_who_share_room_with_user: 2.0 + """ + + def read_config(self, config, **kwargs): + self.event_cache_size = self.parse_size( + config.get("event_cache_size", _DEFAULT_EVENT_CACHE_SIZE) + ) + self.cache_factors = {} # type: Dict[str, float] + + cache_config = config.get("caches") or {} + self.global_factor = cache_config.get( + "global_factor", properties.default_factor_size + ) + if not isinstance(self.global_factor, (int, float)): + raise ConfigError("caches.global_factor must be a number.") + + # Set the global one so that it's reflected in new caches + properties.default_factor_size = self.global_factor + + # Load cache factors from the config + individual_factors = cache_config.get("per_cache_factors") or {} + if not isinstance(individual_factors, dict): + raise ConfigError("caches.per_cache_factors must be a dictionary") + + # Override factors from environment if necessary + individual_factors.update( + { + key[len(_CACHE_PREFIX) + 1 :].lower(): float(val) + for key, val in self._environ.items() + if key.startswith(_CACHE_PREFIX + "_") + } + ) + + for cache, factor in individual_factors.items(): + if not isinstance(factor, (int, float)): + raise ConfigError( + "caches.per_cache_factors.%s must be a number" % (cache.lower(),) + ) + self.cache_factors[cache.lower()] = factor + + # Resize all caches (if necessary) with the new factors we've loaded + self.resize_all_caches() + + # Store this function so that it can be called from other classes without + # needing an instance of Config + properties.resize_all_caches_func = self.resize_all_caches + + def resize_all_caches(self): + """Ensure all cache sizes are up to date + + For each cache, run the mapped callback function with either + a specific cache factor or the default, global one. + """ + for cache_name, callback in _CACHES.items(): + new_factor = self.cache_factors.get(cache_name, self.global_factor) + callback(new_factor) -- cgit 1.4.1 From 4ba55559acf56a041e47ec0d74890d4ad3e0ddb7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 27 May 2020 13:17:01 +0100 Subject: Fix specifying cache factors via env vars with * in name. (#7580) This mostly applise to `*stateGroupCache*` and co. Broke in #6391. --- changelog.d/7580.bugfix | 1 + docs/sample_config.yaml | 6 ++++++ synapse/config/cache.py | 44 +++++++++++++++++++++++++++++++++++++++----- tests/config/test_cache.py | 28 ++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 changelog.d/7580.bugfix (limited to 'synapse/config/cache.py') diff --git a/changelog.d/7580.bugfix b/changelog.d/7580.bugfix new file mode 100644 index 0000000000..b255dc2a12 --- /dev/null +++ b/changelog.d/7580.bugfix @@ -0,0 +1 @@ +Fix specifying individual cache factors for caches with special characters in their name. Regression in v1.14.0rc1. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 0e1be153c7..48f273b0b2 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -643,6 +643,12 @@ caches: # takes priority over setting through the config file. # Ex. SYNAPSE_CACHE_FACTOR_GET_USERS_WHO_SHARE_ROOM_WITH_USER=2.0 # + # Some caches have '*' and other characters that are not + # alphanumeric or underscores. These caches can be named with or + # without the special characters stripped. For example, to specify + # the cache factor for `*stateGroupCache*` via an environment + # variable would be `SYNAPSE_CACHE_FACTOR_STATEGROUPCACHE=2`. + # per_cache_factors: #get_users_who_share_room_with_user: 2.0 diff --git a/synapse/config/cache.py b/synapse/config/cache.py index 91036a012e..acc31652de 100644 --- a/synapse/config/cache.py +++ b/synapse/config/cache.py @@ -14,13 +14,17 @@ # limitations under the License. import os +import re from typing import Callable, Dict from ._base import Config, ConfigError # The prefix for all cache factor-related environment variables -_CACHES = {} _CACHE_PREFIX = "SYNAPSE_CACHE_FACTOR" + +# Map from canonicalised cache name to cache. +_CACHES = {} + _DEFAULT_FACTOR_SIZE = 0.5 _DEFAULT_EVENT_CACHE_SIZE = "10K" @@ -37,6 +41,20 @@ class CacheProperties(object): properties = CacheProperties() +def _canonicalise_cache_name(cache_name: str) -> str: + """Gets the canonical form of the cache name. + + Since we specify cache names in config and environment variables we need to + ignore case and special characters. For example, some caches have asterisks + in their name to donate that they're not attached to a particular database + function, and these asterisks need to be stripped out + """ + + cache_name = re.sub(r"[^A-Za-z_1-9]", "", cache_name) + + return cache_name.lower() + + def add_resizable_cache(cache_name: str, cache_resize_callback: Callable): """Register a cache that's size can dynamically change @@ -45,7 +63,10 @@ def add_resizable_cache(cache_name: str, cache_resize_callback: Callable): cache_resize_callback: A callback function that will be ran whenever the cache needs to be resized """ - _CACHES[cache_name.lower()] = cache_resize_callback + # Some caches have '*' in them which we strip out. + cache_name = _canonicalise_cache_name(cache_name) + + _CACHES[cache_name] = cache_resize_callback # Ensure all loaded caches are sized appropriately # @@ -105,6 +126,12 @@ class CacheConfig(Config): # takes priority over setting through the config file. # Ex. SYNAPSE_CACHE_FACTOR_GET_USERS_WHO_SHARE_ROOM_WITH_USER=2.0 # + # Some caches have '*' and other characters that are not + # alphanumeric or underscores. These caches can be named with or + # without the special characters stripped. For example, to specify + # the cache factor for `*stateGroupCache*` via an environment + # variable would be `SYNAPSE_CACHE_FACTOR_STATEGROUPCACHE=2`. + # per_cache_factors: #get_users_who_share_room_with_user: 2.0 """ @@ -130,10 +157,17 @@ class CacheConfig(Config): if not isinstance(individual_factors, dict): raise ConfigError("caches.per_cache_factors must be a dictionary") + # Canonicalise the cache names *before* updating with the environment + # variables. + individual_factors = { + _canonicalise_cache_name(key): val + for key, val in individual_factors.items() + } + # Override factors from environment if necessary individual_factors.update( { - key[len(_CACHE_PREFIX) + 1 :].lower(): float(val) + _canonicalise_cache_name(key[len(_CACHE_PREFIX) + 1 :]): float(val) for key, val in self._environ.items() if key.startswith(_CACHE_PREFIX + "_") } @@ -142,9 +176,9 @@ class CacheConfig(Config): for cache, factor in individual_factors.items(): if not isinstance(factor, (int, float)): raise ConfigError( - "caches.per_cache_factors.%s must be a number" % (cache.lower(),) + "caches.per_cache_factors.%s must be a number" % (cache,) ) - self.cache_factors[cache.lower()] = factor + self.cache_factors[cache] = factor # Resize all caches (if necessary) with the new factors we've loaded self.resize_all_caches() diff --git a/tests/config/test_cache.py b/tests/config/test_cache.py index b45e0cc536..d3ec24c975 100644 --- a/tests/config/test_cache.py +++ b/tests/config/test_cache.py @@ -126,6 +126,34 @@ class CacheConfigTests(TestCase): add_resizable_cache("foo", cache_resize_callback=cache.set_cache_factor) self.assertEqual(cache.max_size, 150) + def test_cache_with_asterisk_in_name(self): + """Some caches have asterisks in their name, test that they are set correctly. + """ + + config = { + "caches": { + "per_cache_factors": {"*cache_a*": 5, "cache_b": 6, "cache_c": 2} + } + } + t = TestConfig() + t.caches._environ = { + "SYNAPSE_CACHE_FACTOR_CACHE_A": "2", + "SYNAPSE_CACHE_FACTOR_CACHE_B": 3, + } + t.read_config(config, config_dir_path="", data_dir_path="") + + cache_a = LruCache(100) + add_resizable_cache("*cache_a*", cache_resize_callback=cache_a.set_cache_factor) + self.assertEqual(cache_a.max_size, 200) + + cache_b = LruCache(100) + add_resizable_cache("*Cache_b*", cache_resize_callback=cache_b.set_cache_factor) + self.assertEqual(cache_b.max_size, 300) + + cache_c = LruCache(100) + add_resizable_cache("*cache_c*", cache_resize_callback=cache_c.set_cache_factor) + self.assertEqual(cache_c.max_size, 200) + def test_apply_cache_factor_from_config(self): """Caches can disable applying cache factor updates, mainly used by event cache size. -- cgit 1.4.1 From d7d8a2e7ee5058ebc9ce16ca10ecba3e4b1f8928 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 27 May 2020 13:34:46 +0100 Subject: Fix up comments --- docs/sample_config.yaml | 2 +- synapse/config/cache.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'synapse/config/cache.py') diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 48f273b0b2..0ec482719d 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -647,7 +647,7 @@ caches: # alphanumeric or underscores. These caches can be named with or # without the special characters stripped. For example, to specify # the cache factor for `*stateGroupCache*` via an environment - # variable would be `SYNAPSE_CACHE_FACTOR_STATEGROUPCACHE=2`. + # variable would be `SYNAPSE_CACHE_FACTOR_STATEGROUPCACHE=2.0`. # per_cache_factors: #get_users_who_share_room_with_user: 2.0 diff --git a/synapse/config/cache.py b/synapse/config/cache.py index acc31652de..0672538796 100644 --- a/synapse/config/cache.py +++ b/synapse/config/cache.py @@ -46,7 +46,7 @@ def _canonicalise_cache_name(cache_name: str) -> str: Since we specify cache names in config and environment variables we need to ignore case and special characters. For example, some caches have asterisks - in their name to donate that they're not attached to a particular database + in their name to denote that they're not attached to a particular database function, and these asterisks need to be stripped out """ @@ -130,7 +130,7 @@ class CacheConfig(Config): # alphanumeric or underscores. These caches can be named with or # without the special characters stripped. For example, to specify # the cache factor for `*stateGroupCache*` via an environment - # variable would be `SYNAPSE_CACHE_FACTOR_STATEGROUPCACHE=2`. + # variable would be `SYNAPSE_CACHE_FACTOR_STATEGROUPCACHE=2.0`. # per_cache_factors: #get_users_who_share_room_with_user: 2.0 -- cgit 1.4.1