Port cache warming to main branch #21

Closed
jamesarosen opened this Issue Dec 29, 2011 · 16 comments

3 participants

@jamesarosen

@kbuckler I'm happy to port this to master, but I wonder whether a simpler implementation would work:

module FeatureCaching

  def warm_cache
    return unless caches_features?
    all(:order => "id DESC").each do |f|    
      feature_cache.write(f.symbol, f, :expires_in => cache_ttl)
    end
  end

end

And then ask the app to call Feature.warm_cache in an initializer.

@kbuckler
Zendesk member

Not sure I follow. These caches need to be periodically refreshed, else changes won't be picked up in the frontend.

@jamesarosen

They'd still be refreshed on access. If you warm the cache for something, but don't access it in the ttl, do you really want to re-warm it every time? As implemented, I wouldn't call this cache-warming; I'd call it cache-keeping-on-the-burner or cache-sous-vide.

@kbuckler
Zendesk member

Wat, I can't tell if you're being serious or not. :/

The current behavior is as intended. Let's talk tomorrow.

@jamesarosen

Totally serious. This implementation keeps all features in cache at all times (modulo ttl). That's different from cache-warming, which is a technique to get the cache going once at startup, then let it behave normally henceforth.

@kbuckler
Zendesk member
@jamesarosen

I'm not against the idea of Arturo::Feature.keep_all_features_in_cache!; I just want it to be obvious that that's what's happening.

@ghost

According to everything I've heard/read, cache warming is a "on startup" one time operation. It doesnt make sense to me to re-warm once warmed. The cache should "keep itself warm" by simply dumping cached data when it needs space for more actively requested data, but that should be a function of the cache, not the cache warming process.

I totally agree with @jamesarosen's implementation in the initial response. If you need periodic cache refreshing, then I would question your need for a cache at all.

@kbuckler
Zendesk member

@lvanderhoeven I think you're missing part of the picture, we've applied this project at very high scale and found that caching is absolutely necessary and that a chatty cache has poor performance characteristics. This isn't about cache eviction, it's about anticipating the frequency of which the majority of Arturo keys are accessed.

In any case, I don't think @jamesarosen has suggested that we remove the cache component but I'll let him speak to that. We originally anticipated that this is a feature that only some would find useful, which is why it's disabled by default.

@ghost

Heh, I understand what you're asking for. I'm saying cache warming is not going to solve what you are asking for. Also, imo, if you're finding that your cache is "chatty", you have an architecture problem, not a cache problem which is best addressed elsewhere.

@ghost

Obviously, that's my opinion :)

@kbuckler
Zendesk member

Well, the fact is that this is live, proven code. We've been running this (in a very large production deployment) for ~1 year now without issue.

@plukevdh

I maintain that "live, proven code" or "works for me" != correct solution.

@kbuckler
Zendesk member
@jamesarosen

I think the always-hot cache is a good idea, particularly in Arturo 2.0 for env['arturo'].enabled_features. I'll work on a write-through cache implementation for that branch.

@jamesarosen

I just talked with @grosser and @steved555, who handle of lot of our custom Arturo-related code. They informed me that the real problem is making n queries on startup. Allowing rarely-used Features to fall out of cache is fine according to them. I think I'm going to add a preload_all! method to the cache that an app could call in an initializer if it wants instead of adding the "always-keep-the-cache-hot" solution.

@jamesarosen jamesarosen pushed a commit that referenced this issue May 16, 2013
James A. Rosen Add FeatureCaching#warm_cache! [Fixes #21 and #42] 77c700b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment