From d21ffc93f71dec222c6fe1d2f30bdf1dc04e4e2a Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sat, 11 Jul 2026 11:36:12 -0400 Subject: [PATCH] Cache parsed configuration files to avoid redundant reads Every master_config/minion_config/client_config call re-read and re-parsed the configuration file from disk. During master startup this happens many times per process across all forked subprocesses -- a trivial config was read 84 times, and configs with gitfs/ext_pillar remotes far more (the reporter saw 646). Cache the parsed result in _read_conf_file, keyed by the file's mtime and size, and hand callers a deep copy so they cannot corrupt the cache. A changed file (reload) is re-read. This reduces the trivial-config startup read count from 84 to 1. Fixes #59807 --- changelog/59807.fixed.md | 1 + salt/config/__init__.py | 27 ++++++++++ .../unit/config/test_conf_file_cache.py | 53 +++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 changelog/59807.fixed.md create mode 100644 tests/pytests/unit/config/test_conf_file_cache.py diff --git a/changelog/59807.fixed.md b/changelog/59807.fixed.md new file mode 100644 index 000000000000..8ad3083e1397 --- /dev/null +++ b/changelog/59807.fixed.md @@ -0,0 +1 @@ +The master and other daemons now parse each configuration file once and reuse the result while the file is unchanged, instead of re-reading and re-parsing it from disk on every ``master_config``/``minion_config`` call. A trivial master config was read 84 times during startup (far more with gitfs/ext_pillar remotes); it is now read once. diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 3fa73573d724..d6cc183b9b83 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -2045,10 +2045,30 @@ def _append_domain(opts): return "{0[id]}.{0[append_domain]}".format(opts) +# Cache of parsed configuration files keyed by path. Each entry is a +# ``(cache_key, conf_opts)`` tuple where ``cache_key`` is the file's +# ``(st_mtime_ns, st_size)``. The same configuration file is read many times +# while a daemon starts up (see #59807); parsing it once and reusing the result +# avoids that redundant disk I/O and YAML parsing. Callers always receive a deep +# copy so mutating the returned dict cannot corrupt the cache, and a changed +# file (different mtime or size, e.g. a reload) is re-read. +_conf_file_cache = {} + + def _read_conf_file(path): """ Read in a config file from a given path and process it into a dictionary """ + try: + _stat = os.stat(path) + cache_key = (_stat.st_mtime_ns, _stat.st_size) + except OSError: + cache_key = None + if cache_key is not None: + cached = _conf_file_cache.get(path) + if cached is not None and cached[0] == cache_key: + return deepcopy(cached[1]) + log.debug("Reading configuration from %s", path) append_file_suffix_YAMLError = False with salt.utils.files.fopen(path, "r") as conf_file: @@ -2086,6 +2106,13 @@ def _read_conf_file(path): conf_opts["id"] = str(conf_opts["id"]) else: conf_opts["id"] = salt.utils.data.decode(conf_opts["id"]) + + # Cache the successful parse so repeated reads of the same unchanged file + # reuse it. The rename-on-YAMLError case is skipped: it has moved the file + # out from under ``path``, so the stat taken above no longer describes it. + if cache_key is not None and not append_file_suffix_YAMLError: + _conf_file_cache[path] = (cache_key, conf_opts) + return deepcopy(conf_opts) return conf_opts diff --git a/tests/pytests/unit/config/test_conf_file_cache.py b/tests/pytests/unit/config/test_conf_file_cache.py new file mode 100644 index 000000000000..938aa56b5625 --- /dev/null +++ b/tests/pytests/unit/config/test_conf_file_cache.py @@ -0,0 +1,53 @@ +""" +Tests for the ``salt.config._read_conf_file`` parse cache (#59807). +""" + +import salt.config +import salt.utils.files +from tests.support.mock import patch + + +def test_read_conf_file_parses_unchanged_file_once(tmp_path): + """ + ``_read_conf_file`` parses a config file once and reuses the result while + the file is unchanged, so the same file read repeatedly during daemon + startup is not parsed hundreds of times. Regression test for #59807. + """ + conf = tmp_path / "master" + conf.write_text("timeout: 5\n") + salt.config._conf_file_cache.pop(str(conf), None) + with patch("salt.utils.files.fopen", wraps=salt.utils.files.fopen) as fopen_spy: + first = salt.config._read_conf_file(str(conf)) + second = salt.config._read_conf_file(str(conf)) + assert first["timeout"] == 5 + assert second["timeout"] == 5 + # The second read is served from the cache -- the file is opened only once. + assert fopen_spy.call_count == 1 + + +def test_read_conf_file_returns_isolated_copies(tmp_path): + """ + Each call returns a distinct object, so a caller that mutates the result + cannot corrupt the cached copy handed to the next caller. + """ + conf = tmp_path / "master" + conf.write_text("timeout: 5\n") + salt.config._conf_file_cache.pop(str(conf), None) + first = salt.config._read_conf_file(str(conf)) + first["timeout"] = 999 + second = salt.config._read_conf_file(str(conf)) + assert first is not second + assert second["timeout"] == 5 + + +def test_read_conf_file_rereads_changed_file(tmp_path): + """ + A changed file (different size or mtime) invalidates the cache and is + re-read, so a reload is never served stale data. + """ + conf = tmp_path / "master" + conf.write_text("timeout: 5\n") + salt.config._conf_file_cache.pop(str(conf), None) + assert salt.config._read_conf_file(str(conf))["timeout"] == 5 + conf.write_text("timeout: 12345\n") + assert salt.config._read_conf_file(str(conf))["timeout"] == 12345