Repository navigation
fix(cache): concurrent writes of one key neither crash the run nor bill twice - #76
Merged
Merged
Conversation
…ll twice Every write of a key went through the same key.tmp. Two workers answering cases with the same text raced on it, and on Windows the loser raised PermissionError [WinError 32] out of Cache.put, aborting the run and losing every call already paid for (18 of 20 runs in the review's repro). Each write now goes through a temporary file of its own, then os.replace. A write that still fails is counted in the cache stats as write_errors and dropped, since the answer is already in hand; the cost is one more call on a later run, not this run. The runner also holds a lock per cache key across look up, call, and store, so cases sharing a key are answered once per run and the rest are served from the cache instead of each being billed. Different keys never wait on each other. Fixes #27. Co-Authored-By: Claude Opus 5.5 (1M context) <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.
Fixes #27.
The problem. Every write of a cache key went through the same
key.tmp. Two workers answering cases with the same text raced on it, and on Windows the loser raisedPermissionError [WinError 32]out ofCache.put. That aborted the run and lost every call already paid for: 18 of 20 runs in the review's repro.What changed
Cache.putwrites through a temporary file of its own (tempfile.NamedTemporaryFilein the same directory), thenos.replace.write_errorsand dropped instead of raised. The answer is already paid for, so a missing entry costs one more call on a later run, not this run.Checked
mainand pass now:workers=16make exactly one adapter call, with 15 cache hits and no exception. Onmainthis test reproduced theWinError 32crash. It passed 20 runs out of 20.os.replaceraisingPermissionError, the run still returns every record, counts 5write_errors, and leaves no.tmpfiles behind.ruff,mypy --strict, and the fullpytestsuite pass.🤖 Generated with Claude Code