fix(honcho): include file size in memo key to detect sub-second honch… - #47844
Closed
giggling-ginger wants to merge 1 commit into
Closed
fix(honcho): include file size in memo key to detect sub-second honch…#47844giggling-ginger wants to merge 1 commit into
giggling-ginger wants to merge 1 commit into
Conversation
…o.json rewrites The memoization key for _extract_honcho_cache_busting_config was (path, mtime_ns). On most filesystems mtime has 1-second granularity, so two rapid writes within the same second share an identical mtime_ns and the second call returns the stale cached result. Add st_size to the key: when pinPeerName flips (true<->false) the JSON length changes by one byte, so the size changes even when mtime_ns does not. Both values come from a single path.stat() call, so there is no extra I/O cost. Fixes: TestPinTransition::test_cache_busting_signature_reflects_pin_peer_name
Collaborator
Contributor
Author
Thanks for the context, alt-glitch. I see that #46385 covers the same logic. Glad to see the issue is being addressed with the same approach. I'll close this PR. |
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.
What does this PR do?
_extract_honcho_cache_busting_configmemoized its result using(path, mtime_ns)as the cache key. On most filesystems,mtime_nshas 1-second granularity, so rapid consecutive writes tohoncho.jsonwithin the same second would share an identicalmtime_ns. The second call would then hit the stale cache and return the old configuration, causingpinPeerNameflips to be ignored.This PR adds
st_sizeto the memoization key. WhenpinPeerNameflips (true↔false), the serialized JSON changes length by one byte, ensuring the cache key changes even whenmtime_nsremains identical. Sincest_sizeis retrieved during the existingpath.stat()call, there is no additional I/O cost.Related Issue
Fixes #47844
Type of Change
Changes Made
gateway/run.pyin_extract_honcho_cache_busting_config: Includedst_sizein thememo_keytuple to ensure cache invalidation on file content changes that occur within the same second.How to Test
scripts/run_tests.sh tests/honcho_plugin/test_pin_peer_name.py::TestPinTransition::test_cache_busting_signature_reflects_pin_peer_namescripts/run_tests.sh tests/honcho_plugin/test_pin_peer_name.pyChecklist
Code
pytest tests/ -qand all tests passDocumentation & Housekeeping
cli-config.yaml.example— N/ACONTRIBUTING.mdorAGENTS.md— N/Apath.stat()is standard.Screenshots / Logs