search

ngmoco / cache-money

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

Uncacheable for :all #2

Closed mech closed 14 years ago

mech commented 14 years ago

When using simple JobPosting.find(1), the caching always works, but when doing this, it becomes uncacheable.

JobPosting.find(:all) ---- UNCACHEABLE job_postings - {} - {} - {} - {} JobPosting Load (1.6ms) SELECT * FROM job_postings

I see the doc, :all is supported, any obvious thing I have missed?

JobPosting.find(1,2,3) ---- UNCACHEABLE job_postings - {} - {} - {:conditions=>"job_postings.id IN (2,3)"} - {} JobPosting Load (0.5ms) SELECT * FROM job_postings WHERE (job_postings.id IN (2,3))

ngmoco commented 14 years ago

Seriously, that seems ridiculous. With the way cache-money works I don't see it being able to handle a find(:all) query. I will change the docs to say that it is not supported. I would like to support the IN and will try to get to it, but it will be a different issue from this one.

sansari commented 14 years ago

Just a note on this --- it would actually be a bad idea to even add support for find(:all). Remember that if a large dataset is cached, it may be faster to retrieve it from the DB in terms of avoiding a query, but there is a hidden cost.

Take your example. If the SELECT * from job_postings was cached, then the second query you did (with IN (2,3)) would do the following: 1) Pull entire job_postings set from the find(:all) cache 2) Yank out job_postings 2 and 3 from that array, and return them.

This may initially seem like a good idea, but it actually results in terrible performance as the total number of job_postings goes up. This is because now you are converting ALL job_postings in the entire table into AR objects on every query, even when you want just 2. If instead you hit the DB directly, you'd do a query, but it'd be efficient, and you'd only be converting 2 AR objects. This issue will affect any situation where you are caching a large dataset (e.g. using a cache-money index for a query that yields a high # of results that match the given key)

Just wanted to share that finding since it bit me in the ass a year ago when we first started using cache-money.

P.S. Hey ngmoco: thanks for working on this fork. We've been using cache-money for a while now in our app and have hit a few bugs over time. I'm glad to see a fork that is being updated regularly, will be working to integrate it into our app soon. Hopefully that resolves some of em :)

ngmoco commented 14 years ago

No problem. We are trying to keep up with issues on this project. I'm going to start requiring test cases for any fixes that people want us to pull.