Port cache warming to main branch

[jamesarosen]

See #20

[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]

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]

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]

I disagree, cache warming isn't necessarily a one-time event.

But feel free to rename if you think you can come up with a better way to describe this.

On the other hand, altering the behavior here has big performance implications and I'd urge caution before doing so.

On Sunday, March 3, 2013, James Rosen wrote:

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.

— Reply to this email directly or view it on GitHubhttps://github.com/jamesarosen/arturo/issues/21#issuecomment-14360164 .

[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]

@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]

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]

This discussion is no longer on topic nor is it constructive. You feel strongly about this, you should feel free to submit a pull request with the "correct" solution in place.

On Monday, April 1, 2013, Luke van der Hoeven wrote:

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

— Reply to this email directly or view it on GitHubhttps://github.com/jamesarosen/arturo/issues/21#issuecomment-15731147 .

[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.