search

n8 / multi_fetch_fragments

Multi-fetch Fragments makes rendering and caching a collection of template partials easier and faster.
http://ninjasandrobots.com/rails-faster-partial-rendering-and-caching
MIT License
539 stars 63 forks source link

Fix issue where RedisStore is expecting a splatted array #21

Closed cpuguy83 closed 11 years ago

cpuguy83 commented 11 years ago

ActiveSupport::Cache::RedisStore is not expecting an array and throws undefined metod #match for Array

cpuguy83 commented 11 years ago

Well, I thought this was working, but it's not. Just getting '#' for each fragment.

aaronjensen commented 11 years ago

dalli expects a splatted array as well now: https://github.com/mperham/dalli/pull/362

n8 commented 11 years ago

Thanks for bringing that up @aaronjensen. So we should be able to do Rails.cache.read_multi(*mutable_keys) for all cache stores.

I'm still a little confused on using splat. Will this change of slatting the array here in multi_fetch_fragments be backwards compatible with older versions of dalli? I think the answer is yes, but I just want to check...

cpuguy83 commented 11 years ago

So right now the change I made will work fine if it's not already cached, but if it is pulling the cached version a "#" is rendered instead of the fragment, not sure where to go from here.

cpuguy83 commented 11 years ago

Also, check out https://github.com/rails/rails/pull/10234

aaronjensen commented 11 years ago

@n8 it seems that way, as the main change was just to remove a flatten in order to allow arrays as keys: https://github.com/mperham/dalli/pull/362/files#L0L131

DouweM commented 11 years ago

This has been fixed in my pull request #13.

n8 commented 11 years ago

I fixed the splatting in this commit:

https://github.com/n8/multi_fetch_fragments/commit/c0358e462f65d3f4fda9ba6139a1ba68f72551db

I'll push the new Gem up soon.

Sorry for the incredible delay. It's hard managing too many projects! :)

jeffday commented 10 years ago

@n8 any word on a release? I just upgraded Dalli past 2.6.4 and got bit by this issue in production.

n8 commented 10 years ago

@jeffday This was closed 9 months ago and incorporated into the latest release of the gem that has been on RubyGems since July. If there's a problem now with Dalli, it's likely because of a new behavior in Dalli, and is a separate issue from what this was. (even if it turns out to be similar, it would be new news that something is broken)

Can you describe in more detail the issue you saw in production? Was there an error raised or a stack trace you can send over?

jeffday commented 10 years ago

my mistake, i didn't see that 0.0.17 was available, https://github.com/n8/multi_fetch_fragments/commit/c0358e462f65d3f4fda9ba6139a1ba68f72551db fixes the issue.

basically when a version without that commit is used alongside Dalli greater than 2.6.4, Dalli processes the args to read_multi as a single key instead of multiple keys, and automatically truncates it to meet minimum length requirements. the end result is a 0% cache hit rate for anything using this gem.