From 9bac5d62b33f34c25bed27824a12d697b8b6b163 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 27 May 2020 11:44:19 +0100 Subject: [PATCH 1/7] Ensure ReplicationStreamer is always started when replication enabled. (#7579) Fixes #7566. --- changelog.d/7579.bugfix | 1 + synapse/replication/tcp/handler.py | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 changelog.d/7579.bugfix diff --git a/changelog.d/7579.bugfix b/changelog.d/7579.bugfix new file mode 100644 index 000000000..54542b602 --- /dev/null +++ b/changelog.d/7579.bugfix @@ -0,0 +1 @@ +Fix bug where `ReplicationStreamer` was not always started when replication was enabled. Bug introduced in v1.14.0rc1. diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index 03300e533..cbcf46f3a 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -159,6 +159,9 @@ class ReplicationCommandHandler: hs.config.redis_port, ) + # First let's ensure that we have a ReplicationStreamer started. + hs.get_replication_streamer() + # We need two connections to redis, one for the subscription stream and # one to send commands to (as you can't send further redis commands to a # connection after SUBSCRIBE is called). From eefc6b3a0d08cd2a64be7c78c0a4a651cc965be9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 27 May 2020 12:04:37 +0100 Subject: [PATCH 2/7] Don't apply cache factor to event cache. (#7578) This is already correctly done when we instansiate the cache, but wasn't when it got reloaded (which always happens at least once on startup). --- changelog.d/7578.bugfix | 1 + synapse/util/caches/lrucache.py | 4 ++++ tests/config/test_cache.py | 16 ++++++++++++++++ 3 files changed, 21 insertions(+) create mode 100644 changelog.d/7578.bugfix diff --git a/changelog.d/7578.bugfix b/changelog.d/7578.bugfix new file mode 100644 index 000000000..cd2930736 --- /dev/null +++ b/changelog.d/7578.bugfix @@ -0,0 +1 @@ +Fix cache config to not apply cache factor to event cache. Regression in v1.14.0rc1. diff --git a/synapse/util/caches/lrucache.py b/synapse/util/caches/lrucache.py index 29fabac3c..df4ea5901 100644 --- a/synapse/util/caches/lrucache.py +++ b/synapse/util/caches/lrucache.py @@ -81,6 +81,7 @@ class LruCache(object): """ cache = cache_type() self.cache = cache # Used for introspection. + self.apply_cache_factor_from_config = apply_cache_factor_from_config # Save the original max size, and apply the default size factor. self._original_max_size = max_size @@ -294,6 +295,9 @@ class LruCache(object): Returns: bool: Whether the cache changed size or not. """ + if not self.apply_cache_factor_from_config: + return False + new_size = int(self._original_max_size * factor) if new_size != self.max_size: self.max_size = new_size diff --git a/tests/config/test_cache.py b/tests/config/test_cache.py index 292027912..b45e0cc53 100644 --- a/tests/config/test_cache.py +++ b/tests/config/test_cache.py @@ -125,3 +125,19 @@ class CacheConfigTests(TestCase): cache = LruCache(100) add_resizable_cache("foo", cache_resize_callback=cache.set_cache_factor) self.assertEqual(cache.max_size, 150) + + def test_apply_cache_factor_from_config(self): + """Caches can disable applying cache factor updates, mainly used by + event cache size. + """ + + config = {"caches": {"event_cache_size": "10k"}} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + cache = LruCache( + max_size=t.caches.event_cache_size, apply_cache_factor_from_config=False, + ) + add_resizable_cache("event_cache", cache_resize_callback=cache.set_cache_factor) + + self.assertEqual(cache.max_size, 10240) From 4ba55559acf56a041e47ec0d74890d4ad3e0ddb7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 27 May 2020 13:17:01 +0100 Subject: [PATCH 3/7] 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 diff --git a/changelog.d/7580.bugfix b/changelog.d/7580.bugfix new file mode 100644 index 000000000..b255dc2a1 --- /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 0e1be153c..48f273b0b 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 91036a012..acc31652d 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 b45e0cc53..d3ec24c97 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. From d7d8a2e7ee5058ebc9ce16ca10ecba3e4b1f8928 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 27 May 2020 13:34:46 +0100 Subject: [PATCH 4/7] Fix up comments --- docs/sample_config.yaml | 2 +- synapse/config/cache.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 48f273b0b..0ec482719 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 acc31652d..067253879 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 From 4be968d05dde8c79b1a0f21ff2b9d7860419d9a6 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Wed, 27 May 2020 15:52:18 +0300 Subject: [PATCH 5/7] Fix sample config docs error (#7581) 'client_auth_method' commented out value was erronously 'client_auth_basic', when code and docstring says it should be 'client_secret_basic'. Signed-off-by: Jason Robinson --- changelog.d/7581.doc | 1 + docs/sample_config.yaml | 2 +- synapse/config/oidc_config.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 changelog.d/7581.doc diff --git a/changelog.d/7581.doc b/changelog.d/7581.doc new file mode 100644 index 000000000..88beeb7bd --- /dev/null +++ b/changelog.d/7581.doc @@ -0,0 +1 @@ +Fix OIDC client_auth_method commented out value in sample config. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 0ec482719..ce2c23599 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1546,7 +1546,7 @@ oidc_config: # auth method to use when exchanging the token. # Valid values are "client_secret_basic" (default), "client_secret_post" and "none". # - #client_auth_method: "client_auth_basic" + #client_auth_method: "client_secret_basic" # list of scopes to ask. This should include the "openid" scope. Defaults to ["openid"]. # diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index 5af110745..586038078 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -112,7 +112,7 @@ class OIDCConfig(Config): # auth method to use when exchanging the token. # Valid values are "client_secret_basic" (default), "client_secret_post" and "none". # - #client_auth_method: "client_auth_basic" + #client_auth_method: "client_secret_basic" # list of scopes to ask. This should include the "openid" scope. Defaults to ["openid"]. # From b4109499b452887894d514285bdac20869809d0b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 27 May 2020 17:22:28 +0200 Subject: [PATCH 6/7] 1.14.0rc2 --- CHANGES.md | 17 +++++++++++++++++ changelog.d/7578.bugfix | 1 - changelog.d/7579.bugfix | 1 - changelog.d/7580.bugfix | 1 - changelog.d/7581.doc | 1 - synapse/__init__.py | 2 +- 6 files changed, 18 insertions(+), 5 deletions(-) delete mode 100644 changelog.d/7578.bugfix delete mode 100644 changelog.d/7579.bugfix delete mode 100644 changelog.d/7580.bugfix delete mode 100644 changelog.d/7581.doc diff --git a/CHANGES.md b/CHANGES.md index 914690b7b..ac5976871 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,20 @@ +Synapse 1.14.0rc2 (2020-05-27) +============================== + +Bugfixes +-------- + +- Fix cache config to not apply cache factor to event cache. Regression in v1.14.0rc1. ([\#7578](https://github.com/matrix-org/synapse/issues/7578)) +- Fix bug where `ReplicationStreamer` was not always started when replication was enabled. Bug introduced in v1.14.0rc1. ([\#7579](https://github.com/matrix-org/synapse/issues/7579)) +- Fix specifying individual cache factors for caches with special characters in their name. Regression in v1.14.0rc1. ([\#7580](https://github.com/matrix-org/synapse/issues/7580)) + + +Improved Documentation +---------------------- + +- Fix OIDC client_auth_method commented out value in sample config. ([\#7581](https://github.com/matrix-org/synapse/issues/7581)) + + Synapse 1.14.0rc1 (2020-05-26) ============================== diff --git a/changelog.d/7578.bugfix b/changelog.d/7578.bugfix deleted file mode 100644 index cd2930736..000000000 --- a/changelog.d/7578.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix cache config to not apply cache factor to event cache. Regression in v1.14.0rc1. diff --git a/changelog.d/7579.bugfix b/changelog.d/7579.bugfix deleted file mode 100644 index 54542b602..000000000 --- a/changelog.d/7579.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix bug where `ReplicationStreamer` was not always started when replication was enabled. Bug introduced in v1.14.0rc1. diff --git a/changelog.d/7580.bugfix b/changelog.d/7580.bugfix deleted file mode 100644 index b255dc2a1..000000000 --- a/changelog.d/7580.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix specifying individual cache factors for caches with special characters in their name. Regression in v1.14.0rc1. diff --git a/changelog.d/7581.doc b/changelog.d/7581.doc deleted file mode 100644 index 88beeb7bd..000000000 --- a/changelog.d/7581.doc +++ /dev/null @@ -1 +0,0 @@ -Fix OIDC client_auth_method commented out value in sample config. diff --git a/synapse/__init__.py b/synapse/__init__.py index 6327147d8..5e957985d 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -36,7 +36,7 @@ try: except ImportError: pass -__version__ = "1.14.0rc1" +__version__ = "1.14.0rc2" if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)): # We import here so that we don't have to install a bunch of deps when From 4e3a6176358dede0f917e9135af6a87777061f76 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 27 May 2020 17:27:33 +0200 Subject: [PATCH 7/7] Improve changelog wording --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index ac5976871..14a025e03 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,7 +12,7 @@ Bugfixes Improved Documentation ---------------------- -- Fix OIDC client_auth_method commented out value in sample config. ([\#7581](https://github.com/matrix-org/synapse/issues/7581)) +- Fix the OIDC `client_auth_method` value in the sample config. ([\#7581](https://github.com/matrix-org/synapse/issues/7581)) Synapse 1.14.0rc1 (2020-05-26)