ngmoco / cache-money

A Write-Through Cacheing Library for ActiveRecord
Apache License 2.0
161 stars 31 forks source link

find_all_by_ids returns stale records #12

Open orslumen opened 13 years ago

orslumen commented 13 years ago

Hello,

I have a Person model with a name attribute, and that model is cached normally by its id.

When I type in the following I can consistently produce stale results with cache_money: $ script/console

p = Person.find_all_by_id([1, 2]).first p.name => "Al Bundy" p.name = "Alice Bundy" p.save! Person.find_all_by_id([1]).first.name => "Alice Bundy" Person.find_all_by_id([1, 2]).first.name => "Al Bundy"

If you run memcached with verbose on, you will see entries like: <28 get dev_cache:Person:1/id/1/2 <28 add dev_cache:Person:1/id/1/2 0 86400 1062 <28 get dev_cache:Person:1/id/1 <28 add dev_cache:Person:1/id/1 0 86400 729 <28 add dev_cache:lock/Person:1/id/1 0 30 6 <28 set dev_cache:Person:1/id/1 0 86400 948 <28 delete dev_cache:lock/Person:1/id/1 0 <28 get dev_cache:Person:1/id/1/2

28 sending key dev_cache:Person:1/id/1/2

So with find_by_ids([1,2]) the resulting 2 objects are stored together using cache_key "Person:1/id/1/2". And when the name of the Person with ID 1 is updated, only the cache_key with the single object "Person:1/id/1" is updated.

This looks like a major flaw in cache_money. Am I missing something, or do other people experience the same issue?

Kind regards, orslumen.

ashleym1972 commented 13 years ago

I personally think find_by_ids is hosed but.... it's working somewhat for us. We don't see this issue. Can you create a test to expose the flaw?

orslumen commented 13 years ago

Well, we use cache_money together with thinking_sphinx and TS populates the search results using a multiple-ids query. Due to some stale search results I found this issue.

I created a new branch git@github.com:orslumen/cache-money.git and added the spec in spec/cash/write_through_spec.rb#91. There is only 1 commit, so it should be easy to find :]

Here is the test code: it "updates the index caches with multiple ids on save" do story_1 = Story.create!(:title => "I am delicious") story_2 = Story.create!(:title => "I am tasty") Story.get("id/#{story_1.id}").first.title.should == "I am delicious" Story.get("id/#{story_2.id}").first.title.should == "I am tasty" stories_1_and_2 = Story.find_all_by_id([story_1.id, story_2.id]) stories_1_and_2.map(&:title).should == ["I am delicious", "I am tasty"]

  story_1.title = "I am mouthwatering"
  story_1.save!
  Story.get("id/#{story_1.id}").first.title.should == "I am mouthwatering"
  stories_1_and_2 = Story.find_all_by_id([story_1.id, story_2.id])
  stories_1_and_2.map(&:title).should == ["I am mouthwatering", "I am tasty"]
end

Thanks in advance for looking into this.

ashleym1972 commented 13 years ago

I brought that spec into the develop branch. Next time send a pull request please.

orslumen commented 13 years ago

My apologies. This is the 1st time I actually contribute code ;-) so I am not yet familiar with the customs.

ashleym1972 commented 13 years ago

It's all good. That's how we learn.

danielrekshan commented 13 years ago

I'm having some issues with find_all_by_id as well. I'll get "ArgumentError: key too long "sup-development:User:1/id/1/2/3/4/5/6/7/8/9..." once my query gets more than 75 ids or so.

From what I see, it's trying cache the entire query and sets the cache key by concating all the ids in the query. I'm not as concerned with caching the queries as much as the objects themselves.

I found this in production and it replicates in development. I'm using rails 2.3.3.

I think this is the same issue as orslumen's.

Is there a workaround or am I missing something?

Thanks!

orslumen commented 13 years ago

Afaik this is the intended behaviour of cache money. When querying for an array of ids, the result is stored based on a key with all ids. I do not know of any workaround for this using cache money.

We are currently migrating our project to Rails 3 and as cache money does not work there I started developing an alternative. It makes use of multi_read (like 'User:1', 'User:2', etc) to retrieve multiple cached records in one go. The advantage is that find(id1) and find_by_ids(id1, id2, ...) will point to the same cached records (no stale results) and that the cache keys do not get long.

I hope to be able to publish the gem in the coming weeks. Unfortunately it will not work in Rails 2 (out of the box), but it may come in handy when you migrate to Rails 3.

ashleym1972 commented 13 years ago

Yeah, no real workaround at the moment. We are also working on a Rails 3 caching solution if anyone wants to pitch in let me know.

orslumen commented 13 years ago

FYI: I just published a gem that replaces cache_money for Rails 3.0.x, see https://github.com/orslumen/record-cache