`cache_nils` vs `skip_nil`

[andrepcg]

Rails.cache.fetch has a skip_nil attribute that can be passed in. DalliStore is a bit special and uses cache_nils.

How about adding support for skip_nil so it can work nicely with ActiveSupport::Cache::Store?

[petergoldstein]

Since cache_nils is defined on the Client initializer, and skip_nil is defined on fetch they're not quite equivalent, but there should be a configuration that makes things work in a way that's consistent with other stores. Based on my read I don't think any code changes are required.

If I'm reading the CacheStore docs correctly, it looks like there's an expectation that nils are cached by default (the equivalent of setting cache_nils to true in Client initialization or equivalently in the options passed to the cache). Then skip_nil can be used to override that behavior. Since Dalli does not cache nils by default, this leads to behavior that differs.

As the skip_nil option is handled in the ActiveSupport::Cache::Store code, none of the underlying implementations appear to be aware of it. So Dalli is consistent with the other store options in this regard. You just need to set the cache_nils to true in the initialization options.

If I'm getting any of this wrong, please let me know.

[andrepcg]

I think the client initialization should be kept that way, the only thing that (in my mind) could be updated is the fetch implementation to allow a skip_nil. By default, in ActiveSupport, skip_nil is true, meaning that nil are not cached by default, the same as Dalli. I think this is just a nomenclature issue since both arguments do the same thing but reversed.

The issue I'm seeing is that if you're using the Rails cache store, you have to do the following:

Rails.cache.fetch(key, skip_nil: false, cache_nils: true) { nil }

If you do this, it doesn't work with DalliStore. By "doesn't work" I mean the nil won't be cached

Rails.cache.fetch(key, skip_nil: false) { nil }

[petergoldstein]

@andrepcg So this isn't quite right.

Looking at ActiveSupport::Cache::Store you can see a few things:

1. fetch, and only fetch, supports the skip_nil option. These are not supported on the individual read and write methods, as in the context of the Store it's only meaningful on a potentially combined operation (like fetch).
2. That's because Store explicitly states that it stores nil values. You aren't able to change this behavior globally.
3. The Store implementation does not call the Dalli::Client fetch, but rather the individual get and set methods.

In Dalli, cache_nils is used in two ways:

1. Globally, by setting cache_nils: true in the initializer to get Dalli to behave like item 2 in the above.
2. On the fetch method on Dalli::Client, where it allows overriding the global behavior on a case-by-case basis.

So skip_nil and cache_nils are inverses of each other only for the second item in this list. Moreover, changing the behavior in Dalli for that second item will have no impact on the Rails.cache behavior which is what you're trying to effect.

Some things that I think are true:

1. MemCacheStore breaks the Store contract by not enforcing cache_nils: true on the underlying Dalli::Client. I'd consider this a bug in the MemCacheStore implementation. Adding an explicit line here, similar to the line for the compress option would address this and give the expected behavior.
2. It's confusing that cache_nils is passable as a request option through an call to Dalli::Client outside of fetch. As a fetch option it makes sense for the same reason skip_nil makes sense - you're doing what is a combined read/write operation on a key. If this were filtered out of other calls, then the "fix" in your above code wouldn't work, and frankly this would all be less confusing.

Based on the above I stand by my original conclusion - this is a Dalli::Client initialization problem. For the moment you'll need to handle it by passing in cache_nils: true as an option to the initializer. I also think it merits an issue on Rails, and I'll open that and file a PR sometime soon.