Cache parsed configuration files to avoid redundant reads on startup (#59807)

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.

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
 
 

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