artsy / garner

A set of Rack middleware and cache helpers that implement various caching strategies.
MIT License
344 stars 24 forks source link

Mongoid garnered_find causes two similar queries #50

Open dblock opened 11 years ago

dblock commented 11 years ago

Consider Garner::Mixins::Mongoid::Document.garnered_find.

def self.garnered_find(handle)
  return nil unless (binding = identify(handle))
  identity = Garner::Cache::Identity.new
  identity.bind(binding).key({ :source => :garnered_find }) do
    find(handle)
  end
end

The identity.bind(binding) call will cause a database=garner_test collection=mongers selector={"$or"=>[{"_id"=>"m1"}, {"_slugs"=>"m1"}]} flags=[:slave_ok] limit=1 skip=0 batch_size=nil fields={"_id"=>1, "_type"=>1, "updated_at"=>1} query to generate the cache key for this instance.

Then, if we have a cache miss, the second query is placed: database=garner_test collection=mongers selector={"_slugs"=>{"$in"=>["m1"]}} flags=[:slave_ok] limit=1 skip=0 batch_size=nil fields=nil

In this particular scenario it may be more efficient to fetch the entire record in the first query and reuse it as the result, but I am not sure how to express this in code, therefore avoiding the double find.

fancyremarker commented 11 years ago

The fact that identity.bind(binding) generates a database query depends on the configured binding key strategy. For example, CacheKey or SafeCacheKey will generate a database query, but BindingIndex will not.

I think this is best left up to the key strategy, and that we shouldn't go down the path of "peeking into" the code of the cache block (not yet, at least).

fancyremarker commented 11 years ago

Sorry, in my above statement, the BindingIndex example assumed a warm cache. However, it's still possible to construct a strategy in which no database query would be made during identity.bind(binding), even for a cold cache .