fix: clear per-key metadata (hashmapStats) on Reset - #432
Conversation
cacheShard.reset rebuilds hashmap, entryBuffer and the entries queue but leaves hashmapStats untouched. Everywhere else the invariant holds that hashmapStats only contains counts for currently-present entries: del and removeOldestEntry both delete(s.hashmapStats, hash) when an entry leaves the cache. Reset removes every entry yet keeps their request counts, so after Reset() KeyMetadata still reports stale, non-zero RequestCount for keys that are no longer cached, and the map grows without bound across Reset cycles when the key set churns. Re-create hashmapStats in reset(), mirroring initNewShard (zero capacity when StatsEnabled is false so nothing is allocated for the disabled case). Adds TestResetClearsKeyMetadata, which fails before this change (RequestCount == 10 after Reset) and passes after (== 0). Signed-off-by: jaideeppyne <jaideeppyne1997@gmail.com>
|
The red build here is It builds a cache with This PR only touches Happy to push an empty commit if you want CI re-run. |
|
The red This PR only touches |
cacheShard.resetrebuildshashmap,entryBufferand theentriesqueue but leaveshashmapStatsuntouched.Everywhere else the invariant holds that
hashmapStatsonly contains counts for currently-present entries —delandremoveOldestEntrybothdelete(s.hashmapStats, hash)when an entry leaves the cache.Reset()removes every entry yet keeps their request counts, so:Reset(),KeyMetadata(key).RequestCountstill reports a stale, non-zero value for keys that are no longer cached, andResetcycles when the key set churns (a leak).This re-creates
hashmapStatsinreset(), mirroringinitNewShard(zero capacity whenStatsEnabledis false, so nothing is allocated in the disabled case).Adds
TestResetClearsKeyMetadata, which fails before this change (RequestCount == 10afterReset) and passes after (== 0).go test ./...,-race,gofmt, andgo vetall pass.Related: #322 (Reset not clearing stats), #414 (adds a separate
ResetKeyMetadataAPI — orthogonal to this fix).