Shopify/memcached_store

How to get get a default expiration in?

bemurphy opened this issue · 4 comments

I think this is an issue of intercommunication problems between identity cache, memcached_store, and the memcached gem itself.

Given a config like this:

config.identity_cache_store = :memcached_snappy_store, Memcached.new(
  "127.0.0.1",
  support_cas: true,
  auto_eject_hosts: false,
  default_ttl: 13,
)
IdentityCache.cache_backend = ActiveSupport::Cache.lookup_store(*Rails.configuration.identity_cache_store)

the memcached instance will have a default ttl of 13, however in IdentityCache I see this:

def add(key, value)
  @cache_backend.write(key, value, :unless_exist => true) if IdentityCache.should_fill_cache?
end

(https://github.com/Shopify/identity_cache/blob/60f42585a9057a159a39ef09ee22343a3a5fa849/lib/identity_cache/cache_fetcher.rb#L84)

Ultimately this ends up in ActiveSupport:: Cache:: MemcachedStore#expiration:

def expiration(options)
  expires_in = options[:expires_in].to_i
  if expires_in > 0 && !options[:raw]
    # Set the memcache expire a few minutes in the future to support race condition ttls on read
    expires_in += 5.minutes.to_i
  end
  expires_in
end

a value for :expires_in is never passed along, so this is 0.

I think this is an interaction bug and not one of misconfiguration. Does that sound possible? Not sure best how to approach a fix on that (introduce a ttl to IdentityCache, perhaps?)

It's not great, but while trying to find a good fix for this, a workaround could be to create a memory store that inherits and overrides the #expiration method, something like this works:

require "memcached_store"
require "active_support/cache/memcached_snappy_store"

module ActiveSupport
  module Cache
    class MemcachedSnappyStoreWithTtl < ActiveSupport::Cache::MemcachedSnappyStore
      protected

      def expiration(options)
        # Pull the value 42 from configuration
        options = { expires_in: 42 }.merge(options)
        super(options)
      end
    end
  end
end

Turns out the config I was using was simply wrong. This works OK:

```ruby
config.identity_cache_store = :memcached_snappy_store, Memcached.new(
  "127.0.0.1",
  support_cas: true,
  auto_eject_hosts: false,
  default_ttl: 13,
), { expires_in: 42 } # Note the universal options are passed last

Might be good for a readme update somewhere though (here or IdentityCache?)

I suspect most people will stub their toes on this via the IdentityCache readme, so I opened a PR there instead

Closing here since the issue was in the documentation of IdentityCache