Skip to content

Commit

Permalink
Attempt to batch config loading for deployments (#996)
Browse files Browse the repository at this point in the history
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 :(
  • Loading branch information
nemacysts committed Sep 16, 2024
1 parent b6205db commit f7a7b7e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 14 deletions.
28 changes: 24 additions & 4 deletions tron/api/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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.
Expand All @@ -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."""
Expand Down
9 changes: 3 additions & 6 deletions tron/api/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tron/config/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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()


Expand Down

0 comments on commit f7a7b7e

Please sign in to comment.