-
Notifications
You must be signed in to change notification settings - Fork 60
Fix issue where RedisStore is expecting a splatted array #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Well, I thought this was working, but it's not. Just getting '#' for each fragment. |
|
dalli expects a splatted array as well now: petergoldstein/dalli#362 |
|
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... |
|
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. |
|
Also, check out rails/rails#10234 |
|
@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 |
|
This has been fixed in my pull request #13. |
|
I fixed the splatting in this commit: I'll push the new Gem up soon. Sorry for the incredible delay. It's hard managing too many projects! :) |
|
@n8 any word on a release? I just upgraded Dalli past 2.6.4 and got bit by this issue in production. |
|
@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? |
|
my mistake, i didn't see that 0.0.17 was available, c0358e4 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. |
ActiveSupport::Cache::RedisStore is not expecting an array and throws undefined metod #match for Array