Fix specifying cache factors via env vars with * in name. (#7580)

This mostly applise to `*stateGroupCache*` and co.

Broke in #6391.
This commit is contained in:
Erik Johnston 2020-05-27 13:17:01 +01:00 committed by GitHub
parent eefc6b3a0d
commit 4ba55559ac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 74 additions and 5 deletions

1
changelog.d/7580.bugfix Normal file
View file

@ -0,0 +1 @@
Fix specifying individual cache factors for caches with special characters in their name. Regression in v1.14.0rc1.

View file

@ -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

View file

@ -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()

View file

@ -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.