Cache parsed configuration files to avoid redundant reads on startup (#59807)#69779
Open
ggiesen wants to merge 1 commit into
Open
Cache parsed configuration files to avoid redundant reads on startup (#59807)#69779ggiesen wants to merge 1 commit into
ggiesen wants to merge 1 commit into
Conversation
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 saltstack#59807
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Caches parsed configuration files so a given file is read and parsed from disk once while it is unchanged, instead of on every
master_config/minion_config/client_configcall.What issues does this PR fix or reference?
Fixes #59807
Previous Behavior
salt.config._read_conf_filere-read and re-parsed the configuration file from disk on every call. During daemon startup the same file is loaded many times: eachMasterMinionandLocalClientbuilt by_load_modules, across every forked subprocess (Maintenance, MWorkers, ReqServer, ...), calls back throughmaster_config/minion_config->load_config->_read_conf_file. Measured on 3006 with a trivial master config, the file wasopenat()ed 84 times during startup (across 8 processes); the reporter saw 646 with a gitfs/ext_pillar config.New Behavior
_read_conf_filecaches the parsed result keyed by the file's(st_mtime_ns, st_size)and returns a deep copy to each caller. Unchanged files are served from the cache; a changed file (for example a reload) has a different mtime or size and is re-read. The deep copy means callers that mutate the returned opts cannot corrupt the cached copy. With this change the same trivial-config startup reads the file once (84 -> 1), and the master boots and generates its keys normally.Verified:
openat()count for the config file drops from 84 to 1 across a fullsalt-masterstartup (strace).tests/unit/test_config.pystill passes (76 passed, 7 skipped); new tests added.Merge requirements satisfied?
changelog/59807.fixed.mdtests/pytests/unit/config/test_conf_file_cache.py(the parse-once test fails without this change)Commits signed with GPG?
No