-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Reload cache factors from disk on SIGHUP #12673
Changes from 7 commits
d942823
077ce90
1f668be
2a5ce2f
69da4ca
85f4385
1febfd6
4039b2d
b7a6ec5
519f98b
b9595ad
1118b9f
f6e1394
55963ac
f199d82
412414d
a7418c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Synapse will now reload [cache config](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#caching) when it receives a [SIGHUP](https://en.wikipedia.org/wiki/SIGHUP) signal. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,14 +16,18 @@ | |
|
||
import argparse | ||
import errno | ||
import logging | ||
import os | ||
from collections import OrderedDict | ||
from hashlib import sha256 | ||
from textwrap import dedent | ||
from typing import ( | ||
Any, | ||
ClassVar, | ||
Collection, | ||
Dict, | ||
Iterable, | ||
Iterator, | ||
List, | ||
MutableMapping, | ||
Optional, | ||
|
@@ -40,6 +44,8 @@ | |
|
||
from synapse.util.templates import _create_mxc_to_http_filter, _format_ts_filter | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class ConfigError(Exception): | ||
"""Represents a problem parsing the configuration | ||
|
@@ -55,6 +61,38 @@ def __init__(self, msg: str, path: Optional[Iterable[str]] = None): | |
self.path = path | ||
|
||
|
||
def format_config_error(e: ConfigError) -> Iterator[str]: | ||
""" | ||
Formats a config error neatly | ||
|
||
The idea is to format the immediate error, plus the "causes" of those errors, | ||
hopefully in a way that makes sense to the user. For example: | ||
|
||
Error in configuration at 'oidc_config.user_mapping_provider.config.display_name_template': | ||
Failed to parse config for module 'JinjaOidcMappingProvider': | ||
invalid jinja template: | ||
unexpected end of template, expected 'end of print statement'. | ||
|
||
Args: | ||
e: the error to be formatted | ||
|
||
Returns: An iterator which yields string fragments to be formatted | ||
""" | ||
yield "Error in configuration" | ||
|
||
if e.path: | ||
yield " at '%s'" % (".".join(e.path),) | ||
|
||
yield ":\n %s" % (e.msg,) | ||
|
||
parent_e = e.__cause__ | ||
indent = 1 | ||
while parent_e: | ||
indent += 1 | ||
yield ":\n%s%s" % (" " * indent, str(parent_e)) | ||
parent_e = parent_e.__cause__ | ||
|
||
|
||
# We split these messages out to allow packages to override with package | ||
# specific instructions. | ||
MISSING_REPORT_STATS_CONFIG_INSTRUCTIONS = """\ | ||
|
@@ -119,7 +157,7 @@ class Config: | |
defined in subclasses. | ||
""" | ||
|
||
section: str | ||
section: ClassVar[str] | ||
|
||
def __init__(self, root_config: "RootConfig" = None): | ||
self.root = root_config | ||
|
@@ -309,9 +347,12 @@ class RootConfig: | |
class, lower-cased and with "Config" removed. | ||
""" | ||
|
||
config_classes = [] | ||
config_classes: List[Type[Config]] = [] | ||
|
||
def __init__(self, config_files: Collection[str] = ()): | ||
# Capture absolute paths here, so we can reload config after we daemonize. | ||
self.config_files = [os.path.abspath(path) for path in config_files] | ||
|
||
def __init__(self): | ||
for config_class in self.config_classes: | ||
if config_class.section is None: | ||
raise ValueError("%r requires a section name" % (config_class,)) | ||
|
@@ -512,12 +553,10 @@ def load_config_with_parser( | |
object from parser.parse_args(..)` | ||
""" | ||
|
||
obj = cls() | ||
|
||
config_args = parser.parse_args(argv) | ||
|
||
config_files = find_config_files(search_paths=config_args.config_path) | ||
|
||
obj = cls(config_files) | ||
if not config_files: | ||
parser.error("Must supply a config file.") | ||
|
||
|
@@ -627,7 +666,7 @@ def load_or_generate_config( | |
|
||
generate_missing_configs = config_args.generate_missing_configs | ||
|
||
obj = cls() | ||
obj = cls(config_files) | ||
|
||
if config_args.generate_config: | ||
if config_args.report_stats is None: | ||
|
@@ -727,6 +766,20 @@ def generate_missing_files( | |
) -> None: | ||
self.invoke_all("generate_files", config_dict, config_dir_path) | ||
|
||
def reload_config_section(self, section_name: str) -> None: | ||
"""Reconstruct the given config section, leaving all others unchanged. | ||
|
||
:raises ValueError: if the given `section` does not exist. | ||
:raises ConfigError: for any other problems reloading config. | ||
""" | ||
existing_config: Optional[Config] = getattr(self, section_name, None) | ||
if existing_config is None: | ||
raise ValueError(f"Unknown config section '{section_name}'") | ||
logger.info("Reloading config section '%s'", section_name) | ||
|
||
new_config_data = read_config_files(self.config_files) | ||
existing_config.read_config(new_config_data) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if we error half way through reading the config? I.e. if someone has put an invalid value in there? Do we end up in a messed up state where we've reloaded some of the config? I guess ideally we should be reading the config into a new config class and then replacing the existing config with that once we've successfully read it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We will, yes. I don't think that's the end of the world, because you can correct and re-reload the config. But it would be nice for this to be an atomic operation. I'll do this shortly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note however that the CacheConfig in particular is naughty. When it reads in config it adjusts global state in its enclosing module. And it automatically re-evaluates cache sizes. I'll have to change this too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh that's annoying :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've posted four commits which have a go at this. Could use somebody to sanity check that it works though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The reloading seems to work fine. Not sure how to check that the caches themselves have the appropriate size from the diffs? |
||
|
||
|
||
def read_config_files(config_files: Iterable[str]) -> Dict[str, Any]: | ||
"""Read the config files into a dict | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring pwease?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
412414d