API shortcoming on nil values

[karalabe]

We've been integrating fastcache into our project and @holiman found an interesting corner case that fastcache handles correctly internally, but fails to properly surface it to the user.

In our specific use case, values associated to keys can be empty. So, what we would like to do is insert key->nil mappings and be able to retrieve them. This works half way:

cache := fastcache.New(500 * 1024)

fmt.Println(cache.Has([]byte("a")))
cache.Set([]byte("a"), nil)
fmt.Println(cache.Has([]byte("a")))

^ correctly returns false and then true.

---

However, if I use Get, things don't look so well:

cache := fastcache.New(500 * 1024)

fmt.Println(cache.Get(nil, []byte("a")) == nil)
cache.Set([]byte("a"), nil)
fmt.Println(cache.Get(nil, []byte("a")) == nil)

^ returns nil in both cases. You could even set []byte{} and both would still return nil.

---

Why is this an issue? Because we cannot differentiate between a value being inside-the-cache-and-empty, or a value missing-from-the-cache requiring database access. The current API only allows using Has-then-Get to cover this, but that's not thread safe any more.

The fault is a combination of https://github.com/VictoriaMetrics/fastcache/blob/master/fastcache.go#L386, because Go's append will keep dst as nil if appending an empty slice; and https://github.com/VictoriaMetrics/fastcache/blob/master/fastcache.go#L161, because it discards the found flag and only returns dst.

One solution would be to change the append line so it special cases the scenario where both dst and the found value is nil, but that won't really work if the user does supply a non-nil dst.

Our best solution until now is to add a second Get method that returns not only the cached value, but also the flag whether it was found or not. This way we don't break the API.

---

Does this seem good to you? We can open a PR if you want.

[valyala]

Our best solution until now is to add a second Get method that returns not only the cached value, but also the flag whether it was found or not. This way we don't break the API.

This sounds like the best solution.

[valyala]

Published v1.5.3 with the merged PR that adds Cache.HasGet method.