Repository navigation
Update cache file access time on cache hit so eviction keeps the most… - #104
Closed
ryanhaguematomo wants to merge 3 commits into
Closed
ryanhaguematomo wants to merge 3 commits into
ryanhaguematomo wants to merge 3 commits into
Conversation
… used entries deleteLeastAccessedFiles() evicts by fileatime(), but a cache hit never updated it (opcache, noatime/relatime mounts), so eviction effectively dropped the oldest written entries. Cache hits now move the access time, at most once per day per file to keep tracking writes low. WarmDeviceDetectorCacheTest asserted the inverse behaviour and was flaky because entries written within the same second tied on fileatime(). The test now ages the entries first and asserts the most used entry is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ate directly touch() with explicit times requires owning the file, so the tracker could not update the access time when the warm command runs as another user. touch() without times only needs write permission. The access time tests now call markAsAccessed() directly, since reading the file in getCached() lets relatime mounts update the access time on their own and hide a regression. The README documents the eviction behaviour and the required permissions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.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.
Draft state:
#84 seemingly attempted to resolve the same issue but was closed with merge. I cant find documentation as to why this was done, chasing it now to avoid any learned issues.
Description to be updated prior to making ready
This PR
deleteLeastAccessedFiles() evicts by fileatime(), but a cache hit never updated it (opcache, noatime/relatime mounts), so eviction effectively dropped the oldest written entries. Cache hits now move the access time, at most once per day per file to keep tracking writes low.
WarmDeviceDetectorCacheTest asserted the inverse behaviour and was flaky because entries written within the same second tied on fileatime(). The test now ages the entries first and asserts the most used entry is kept.
Description
This PR aims to resolve the flaky test.
Checklist