fix(ttlcache): refresh clobber, deadlocks and ttl precision - #1
Open
zze0s wants to merge 29 commits into
Open
Conversation
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
GetTTL refresh clobbering a concurrentSet— the refresh now re-reads under the write lock instead of writing back a stale copySetdeadlocking against the expiration loop on a full wake channel — the loop is now driven by a coalesced signal plus an earliest-deadline field guarded by the lock, so a dropped signal can never lose or delay an expirationGet,GetOrSet,Keys,All) instead of being returned, or resurrected by the sliding refresh, while the sweep lagged; a store or delete landing on such an entry still hands it to the deallocation callback as timed outClosereturning while the expiration goroutine was still running — it now waits for the loop, including any in-flight deallocation callbackttlcachenow stamps and sweeps with realtime.Now()and no longer usestimecache(a 200ms item in aSetDefaultTTL(10s)cache lived 2.6–5.2s, now ~201ms);Getrefreshes are batched to the resolution, capped at half the item's TTLSetDefaultTTL,SetTimerResolution,DisableUpdateTime,SetDeallocationFunconttlcache,RoundontimecacheKeys()/All()iterators;GetKeysstays as a deprecated shimSetover a live key silently dropping the displaced value — the deallocation callback now fires withReasonReplaced, andGetOrSet's bool now reports whether the key was already present (it was alwaystruebefore), so callers can release the loser of a store racetesting/synctest, and gate the suite on the Go 1.27goroutineleakprofilego test,go test -raceWhy
Closereturned before the loop stoppedGetOrSetchanges close a prepared-statement leak in qui'sdatabase/sqlcaches ahead of its migration to this moduleBreaking
Optionsbuilders no longer compiles — pass the functional options toNewinstead:ttlcache.New(ttlcache.SetDefaultTTL(...)),timecache.New(timecache.Round(...))GetKeysis deprecated but keeps working — migrate toKeys()when convenientGetOrSet/GetOrSetItemreturntrueonly when the key was already present (previously alwaystrue), and aSetover an existing key now runs the deallocation callback for the displaced valueClosenow blocks until the expiration goroutine exits; calling it from a timeout deallocation callback would deadlock (previously it returned immediately)Testing
go test -race ./...green; the timing tests are deterministic under synctestGet/GetItem/GetKeys/All, corpse displacement reasons, andClosereturning earlygoroutineleakprofile; the pipeline was validated against deliberately leaked caches