refactor: generic rwmutex cache for ip and txt#34
Open
lidel wants to merge 2 commits into
Open
Conversation
Separate expired-entry deletion from reads so the ip and txt caches can share one generic cache[V] guarded by a read-write mutex. - a read takes the read lock, so cache hits run in parallel - get deletes an expired entry under the write lock: it drops the read lock, takes the write lock, and re-checks the entry first, so a value a concurrent set refreshed in the gap is kept - a zero TTL stores nothing, so a disabled cache stays empty - tests cover expiry, the zero-TTL no-op, and concurrent access closes #5
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
1 participant
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
The resolver caches sit behind a single
sync.Mutex, so every lookup, including a read-only cache hit, serializes on it. A plainRWMutexwould let reads run in parallel, but the version that did (before #4) deleted expired entries while holding only the read lock, a data race. That race is why #4 fell back to the single mutex, leaving reads serialized.Fix
getdrops the read lock, takes the write lock, and re-checks the entry before deleting, so a concurrentsetthat refreshed it is not clobbered (rationale in thegetcomment inresolver.go)cache[V]holds this logic for both the IP and TXT cachesReads run concurrently again while expired-entry cleanup stays race-free. vyzo's suggestion in #5 to queue expired entries for a separate write-locked cleanup pass is not needed here:
getalready takes the write lock on the rare expired hit, so it deletes that one entry inline instead of maintaining a cleanup queue. Closes #5.