Cache the keycache location resolution instead of redoing it per operation#223
Open
djw8605 wants to merge 1 commit into
Open
Cache the keycache location resolution instead of redoing it per operation#223djw8605 wants to merge 1 commit into
djw8605 wants to merge 1 commit into
Conversation
…ation resolve_cache_location() was called for every keycache operation -- at least once per token validation -- and each call performed getpwuid_r, two mkdir calls, and a full SQLite open / CREATE TABLE / close round trip via initialize_cachedb(), after which the caller opened the database a second time to do actual work. Cache the successful resolution behind a generation counter that configuration changes affecting the location (keycache.cache_home, keycache.allow_in_memory) bump for invalidation. Failed resolutions are not cached, so transient permission/disk problems are retried. One behavior note: a change to after the first resolution is no longer picked up automatically (a cache-home config change still invalidates); environment mutation mid-process was of dubious reliability before. Also set the SQLite busy timeout in get_all_issuers_from_db() -- the only DB path without it -- so a concurrent write no longer makes the background refresher silently see a partial issuer list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Problem
resolve_cache_location()runs for every keycache operation — at least once per token validation, twice for validations that also store keys. Each call performs:getpwuid_r(password-database lookup),mkdircalls,CREATE TABLE IF NOT EXISTS/ close round trip (initialize_cachedb),…after which the caller opens the database a second time to do the actual query. That's two SQLite opens plus a DDL statement on the hot path of every validation, even with a warm cache.
Fix
resolve_cache_location_uncached()and a caching wrapper.keycache.cache_home,keycache.allow_in_memory) bump.Also sets the SQLite busy timeout in
get_all_issuers_from_db()— the only DB path missing it — so a concurrent write no longer makes the background refresher silently return a partial issuer list.Behavior note
A change to
$XDG_CACHE_HOMEafter the first resolution is no longer picked up automatically (settingkeycache.cache_homestill invalidates immediately). Mutating the environment mid-process was of dubious reliability before.Testing
ctestunit, env_config, and monitoring suites pass — includingSetGetConfiguredCacheHome/GetKeycacheLocation, which change the cache home mid-run and exercise the invalidation path.🤖 Generated with Claude Code