Skip to content

Commit

Permalink
Make sure we completely evict entries from the cache
Browse files Browse the repository at this point in the history
Entries were not evicted from the composed cache when an entry was
supposed to be evicted to make room for a newly added entry.

E.g., we composed an LRU cache with a 10 entries limit, with the
per-item TTL cache. When we already had 10 entries in the composed
cache and we wanted to add a new one, we needed to evict one of the
existing entries. The issue was that after adding the new entry, if
you "dumped" the contents of the cache (e.g., by using `prn`), you
didn't see the entry that was supposed to have been evicted. If you
called `cache/lookup` or tried to access the entry using map notation,
you didn't get the entry (if you used the 3-arity versions of the
functions, you were told that there was no such entry). But if you
called `cache/has?` with that key, you were told that the entry *was*
in the cache! (when it was supposed to *not* be there).

We were not removing the entry from the expiry-heap. Which worked
mostly OK when we used the cache in a stand-alone configuration, but
produced the pathological behaviour described above when used in a
composed cache.
  • Loading branch information
iarenaza committed Apr 7, 2021
1 parent 6e24bf7 commit 424e124
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/coop/magnet/ttlcache.clj
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@
(into (priority-map) (for [x base] [(key x) (+ now (get-ttl (key x) (val x)) )]))
get-ttl)))
(evict [_ key]
(PerItemTTLCache. (dissoc cache key)
expiry-heap
get-ttl))
(PerItemTTLCache. (dissoc cache key)
(dissoc expiry-heap key)
get-ttl))
Object
(toString [_]
(str cache \, \space expiry-heap)))
Expand Down

0 comments on commit 424e124

Please sign in to comment.