rollout icon indicating copy to clipboard operation
rollout copied to clipboard

Should memoization be used?

Open berniechiu opened this issue 5 years ago • 5 comments

Description

I found the feature setting is retrieved directly from redis often if used on every request. Should we memoize it?

berniechiu avatar Jan 30 '20 04:01 berniechiu

We noticed this as well and ended up subclassing the Rollout class to add request-level memoization via the RequestStore gem. There's a probably a better way to do it but this worked well enough for us.

class MemoizedRollout < Rollout
  def active?(feature, user = nil)
    key = key_for(feature, user)
    RequestStore.fetch(key) { super }
  end

  def activate(feature)
    clear_cache(feature)
    super
  end

  def deactivate(feature)
    clear_cache(feature)
    super
  end

  def activate_user(feature, user)
    clear_cache(feature)
    super
  end

  def deactivate_user(feature, user)
    clear_cache(feature)
    super
  end

  private

  def clear_cache(feature)
    RequestStore.store.keys.each do |key|
      next unless key.to_s.start_with?("rollout:#{feature}")
      RequestStore.delete(key)
    end
  end

  def key_for(feature, user)
    ['rollout', feature.to_s, user&.class&.to_s, user&.id].compact.join(':')
  end
end

jordanfbrown avatar Feb 04 '20 04:02 jordanfbrown

Cool! @jordanfbrown, thanks for sharing!

berniechiu avatar Feb 04 '20 07:02 berniechiu

Rollout isn’t overly public about it but I believe it uses the adapter pattern by only using like two of Redis methods. You can inject anything you like which means you can make a class with same interface that wraps the Redis client with memorization.

jnunemaker avatar Feb 04 '20 13:02 jnunemaker

Kind what @jordanfbrown did but you shouldn’t need to subclass rollout. You can just define the same interface and initialize with a Redis instance. Your class could be MemoizedRedis. Then you just pass that into Rollout.new. The only other issue with memoization is to turn it on per request or job and ensure it is cleared after. You can do this with middleware or in before action calls in rails. Middleware early in the stack is best.

jnunemaker avatar Feb 04 '20 13:02 jnunemaker

@jnunemaker Ha thanks!

I was just thinking about how https://github.com/jnunemaker/flipper works when opened the issue here

berniechiu avatar Feb 10 '20 06:02 berniechiu