From f7a7b7e062a32f3afcd3db735255ab06f192f3d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20P=C3=A9rez?= Date: Mon, 16 Sep 2024 12:03:14 -0400 Subject: [PATCH] Attempt to batch config loading for deployments (#996) Right now we make at most 2N calls to the Tron API during config deployments: N to get the current configs and at most N if all services have changes. To start, I'd like to reduce this to N by allowing GET /api/config to return all the configs so that the only requests needed are POSTs for changed configs. Depending on how this goes, we can look into batching up the POSTs so that we can also do that in a single request. In terms of speed, it looks like loading all the configs from pnw-prod (on my devbox) with this new behavior takes ~3s - which isn't great, but there's a decent bit of file IO going on here :( --- tron/api/controller.py | 28 ++++++++++++++++++++++++---- tron/api/resource.py | 9 +++------ tron/config/manager.py | 8 ++++---- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/tron/api/controller.py b/tron/api/controller.py index add637333..85b605c05 100644 --- a/tron/api/controller.py +++ b/tron/api/controller.py @@ -2,8 +2,11 @@ Web Controllers for the API. """ import logging +from typing import Dict +from typing import TypedDict from tron import yaml +from tron.config.manager import ConfigManager from tron.eventbus import EventBus log = logging.getLogger(__name__) @@ -175,6 +178,11 @@ def handle_command(self, command, run_time=None): ) +class ConfigResponse(TypedDict): + config: str + hash: str + + class ConfigController: """Control config. Return config contents and accept updated configuration from the API. @@ -184,17 +192,29 @@ class ConfigController: def __init__(self, mcp): self.mcp = mcp - self.config_manager = mcp.get_config_manager() + self.config_manager: ConfigManager = mcp.get_config_manager() - def _get_config_content(self, name): + def _get_config_content(self, name) -> str: if name not in self.config_manager: return self.DEFAULT_NAMED_CONFIG return self.config_manager.read_raw_config(name) - def read_config(self, name): + def read_config(self, name) -> ConfigResponse: config_content = self._get_config_content(name) config_hash = self.config_manager.get_hash(name) - return dict(config=config_content, hash=config_hash) + return {"config": config_content, "hash": config_hash} + + def read_all_configs(self) -> Dict[str, ConfigResponse]: + configs = {} + + for service in self.config_manager.get_namespaces(): + config: ConfigResponse = { + "config": self._get_config_content(service), + "hash": self.config_manager.get_hash(service), + } + configs[service] = config + + return configs def check_config(self, name, content, config_hash): """Update a configuration fragment and reload the MCP.""" diff --git a/tron/api/resource.py b/tron/api/resource.py index d923e5b05..663f4ed10 100644 --- a/tron/api/resource.py +++ b/tron/api/resource.py @@ -385,12 +385,9 @@ def get_config_index(self): def render_GET(self, request): config_name = requestargs.get_string(request, "name") if not config_name: - return respond( - request=request, - response={"error": "'name' for config is required."}, - code=http.BAD_REQUEST, - ) - response = self.controller.read_config(config_name) + response = self.controller.read_all_configs() + else: + response = self.controller.read_config(config_name) return respond(request=request, response=response) @AsyncResource.exclusive diff --git a/tron/config/manager.py b/tron/config/manager.py index 893a805a0..21b6e19ca 100644 --- a/tron/config/manager.py +++ b/tron/config/manager.py @@ -35,7 +35,7 @@ def write_raw(path, content): fh.write(maybe_decode(content)) -def read_raw(path): +def read_raw(path) -> str: with open(path) as fh: return fh.read() @@ -98,7 +98,7 @@ def build_file_path(self, name): name = name.replace(".", "_").replace(os.path.sep, "_") return os.path.join(self.config_path, "%s.yaml" % name) - def read_raw_config(self, name=schema.MASTER_NAMESPACE): + def read_raw_config(self, name=schema.MASTER_NAMESPACE) -> str: """Read the config file without converting to yaml.""" filename = self.manifest.get_file_name(name) return read_raw(filename) @@ -157,7 +157,7 @@ def load(self): name_mapping = self.get_config_name_mapping() return config_parse.ConfigContainer.create(name_mapping) - def get_hash(self, name): + def get_hash(self, name) -> str: """Return a hash of the configuration contents for name.""" if name not in self: return self.DEFAULT_HASH @@ -166,7 +166,7 @@ def get_hash(self, name): def __contains__(self, name): return name in self.manifest - def get_namespaces(self): + def get_namespaces(self) -> str: return self.manifest.get_file_mapping().keys()