Merge tag 'v1.14.0rc2' into develop

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](#7578))
- Fix bug where `ReplicationStreamer` was not always started when replication was enabled. Bug introduced in v1.14.0rc1. ([\#7579](#7579))
- Fix specifying individual cache factors for caches with special characters in their name. Regression in v1.14.0rc1. ([\#7580](#7580))

Improved Documentation
----------------------

- Fix the OIDC `client_auth_method` value in the sample config. ([\#7581](#7581))

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 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)
 ==============================
 

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.0`.
+   #
    per_cache_factors:
      #get_users_who_share_room_with_user: 2.0
 
@@ -1540,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"].
     #

synapse/__init__.py

@@ -36,7 +36,7 @@
 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

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 @@ def __init__(self):
 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 denote 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 @@ def generate_config_section(self, **kwargs):
            # 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.0`.
+           #
            per_cache_factors:
              #get_users_who_share_room_with_user: 2.0
         """
@@ -130,10 +157,17 @@ def read_config(self, config, **kwargs):
         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 @@ def read_config(self, config, **kwargs):
         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()

synapse/config/oidc_config.py

@@ -112,7 +112,7 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
             # 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"].
             #

synapse/replication/tcp/handler.py

@@ -159,6 +159,9 @@ def start_replication(self, hs):
                 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).

synapse/util/caches/lrucache.py

@@ -81,6 +81,7 @@ def __init__(
         """
         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 @@ def set_cache_factor(self, factor: float) -> bool:
         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

tests/config/test_cache.py

@@ -125,3 +125,47 @@ def test_global_instantiated_after_config_load(self):
         cache = LruCache(100)
         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.
+        """
+
+        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)