[wip] PKI index optimization using memory-mapped hash table#68891
Open
dwoz wants to merge 18 commits intosaltstack:masterfrom
Open
[wip] PKI index optimization using memory-mapped hash table#68891dwoz wants to merge 18 commits intosaltstack:masterfrom
dwoz wants to merge 18 commits intosaltstack:masterfrom
Conversation
dwoz
changed the title
Introduce a generic memory-mapped cache utility to eliminate $O(N)$ directory scans when verifying minion IDs on the Master. - Create salt/utils/mmap_cache.py for high-performance lockless lookups - Add Master configuration options for PKI index management - Refactor CkMinions to utilize the mmap index for target verification - Update Maintenance daemon to synchronize the index via the event bus - Implement pki.rebuild_index runner for manual index maintenance - Add comprehensive unit tests and Sphinx documentation
Improve system performance by integrating an O(1) memory-mapped index into the cache driver and CLI. This change ensures minion lookups are near-instant and reduces Master FD pressure. - Update cache driver to atomically maintain index - Fully utilize index in Key and CkMinions classes - Shift system verification to tracking actual FD usage - Optimize mmap_cache with sparse files and bulk rebuilds - Cleanup unused tests and imports
Align PKI index slot_size with Salt's 128-byte default to prevent IndexError during lookups. - Synchronize slot_size to 128 in localfs_key and PkiIndex - Fix hardcoded defaults to match salt.config
- Fix list_all signature mismatch in localfs_key - Add file size verification to MmapCache.open() - Ensure mmap is closed after every operation to prevent FD leaks - Update verify_env to ensure salt user owns index files - Fix salt-key fallback to directory scan when index is missing
- Handle cluster_pki_dir in localfs_key and PkiIndex - Ensure dotfiles like .pki_index.mmap are chowned in verify_env - Prevent FD leaks by closing mmap after every operation
Fix three critical issues causing package install/upgrade test failures: 1. Clustered environment PKI path bug in rebuild_index() When cluster_id is configured, rebuild_index() was incorrectly using pki_dir instead of cluster_pki_dir, causing the index to be built from the wrong location and breaking minion key authentication. 2. Missing salt.output import in salt/key.py Added missing import that caused UnboundLocalError when using salt-key with certain output formatters. 3. Robust error handling for list_status() in CkMinions Added null check for list_status() return value before accessing 'minions' key, preventing AttributeError when key listing fails. These fixes resolve the "No minions matched the target" errors that were causing widespread package test failures across all platforms in CI (Debian, Ubuntu, Rocky Linux, macOS, Photon OS).
- Fix nested dictionary bug in _check_glob_minions that broke compound matching - Fix UnboundLocalError in salt/key.py by using aliased local imports - Fix clustered environment path selection bug in valid_id() - Restore required cachedir parameter in list_all() for stability
- Use explicit write() instead of truncate() for robust file allocation - Add flush() calls after mmap writes for better cross-process visibility - Ensure all local salt.* imports in salt/key.py use aliases to prevent shadowing - Correct nested dictionary bug in _check_glob_minions - Fix clustered environment path selection in valid_id()
…ssions - Use _indices dict in localfs_key to isolate index objects by pki_dir - Ensure PKI index files have 0600 permissions in verify_env - Move index ownership/permission logic to end of verify_env for efficiency
- Remove expensive open/close on every operation - Use salt.utils.files.wait_lock for multi-process safe writes - Use realpath for path comparisons in index caching - Use explicit write() instead of truncate() for robust file allocation - Ensure PKI index files have 0600 permissions
- Remove expensive open/close on every operation for better performance - Use fcntl.flock for multi-process safe writes without deadlock risks - Robustly handle atomic file swaps in open() - Ensure full file allocation during initialization for macOS/Windows compatibility
- Move .pki_index.mmap from /etc to /var/cache/salt/master - Update verify_env signature to accept opts and handle permissions in new location - Update all verify_env call sites to pass opts - Fix NameError in verify_env and PermissionError in unit tests
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.
Introduce a generic memory-mapped cache utility to eliminate$O(N)$ directory scans when verifying minion IDs on the Master.
What does this PR do?
What issues does this PR fix or reference?
Fixes
Previous Behavior
Remove this section if not relevant
New Behavior
Remove this section if not relevant
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes/No