monix / shade

Memcached client for Scala
MIT License
106 stars 19 forks source link

Shade version 2.0 - should we drop the in-memory cache? #22

Closed alexandru closed 8 years ago

alexandru commented 8 years ago

One thing I don't like about Shade is its in-memory cache.

That was a sloppy job, as I needed it at some point, but I think much better implementations are possible and already exist. For example the one from Guava or Ehcache (I've used neither, this is just based on a quick search). The idea of the in-memory cache was to have a mutable Map where keys have an expiration date. Towards that purpose it does a reasonable job, but there are problems:

  1. a true cache will have a secondary eviction policy, like based on usage (LRU), or size and currently our implementation has no such eviction policy, not even something dumb like discarding based on expiry date and a maximum number of items
  2. when fetching things from RAM, it does not make sense to return Future results; there's nothing more synchronous then fetching things from memory and wrapping those results in a Future, pretending that you're doing the user a favor, is clearly a mistake

So maybe we should just drop the in-memory cache thing. Or maybe base it on something existing that does a better job, but if we do that, then what value do we add on top, as integration with Scala's Future will not be it.

lloydmeta commented 8 years ago

I personally like the in-memory cache, although I agree with your concern about not having a secondary eviction policy.

It's really helpful for things like testing, and as a fall-back. This is only possible because it has the same interface as the normal MemcachedImpl. As a result, I really think number 2 is not a problem: such an in-memory cache is helpful because it implements the same interface (returns Future results).

I think the in-memory cache should be kept, but perhaps use Guava to back it, and still have it wrap results with Future.

One possibility might be to release it as a separate "support" lib (e.g. com.bionicspirit.shade-inmemory), so that people who don't want it can choose not to have it. It also opens the door up to splitting it off easier in the future in case we decide we no longer want to support it.

alexandru commented 8 years ago

We aren't dropping it. Moving on.