3.2 stable marshalling #4196

Merged
merged 1 commit into from Dec 28, 2011

Projects

None yet

4 participants

@gazay

It's relates to issue #4042

In rails 3.1 and earlier values in entries did not marshaled without options[:compress] and if it's Numeric.
But after commit a263f37 all values in entries are marshalled. And after that code in ActiveSupport::Cache::Entry#value expects that all values in entries will be marshaled (and will be strings). So here we need a check for old ones.

Should I make a patch for master?

@juwalter

Hi,

not sure if this is the best way to go about it - I thought about it a little and I think that it is a good idea to "marshal" all values in Cache::Entry and let the actual CacheStore implementation take care of serialization.

That would mean, that Cache::FileStore should not do Marshal.load and Marshal.dump - because this is actually not necessary (since @value has already been marshaled by Cache::Entry) any more, only serialization should be done by the cache store implementation (see memcache client, and dalli store).

Also, ZLib::Inflate/Deflate should be performed after a value has been marshaled.

What do you think?

@gazay

I think in file store marshalling it is something like serialization for entries. Yes, value of entry marshalled inside entry, but to write entry in file and keep it there - convert it into byte stream - not bad idea.

And about my commit - I mixed in my head about rails versions - my commit will not work on 1.8.7 because of encoding. So here is the other idea how to handle it.

@tenderlove tenderlove merged commit df37018 into rails:3-2-stable Dec 28, 2011
@tenderlove
Ruby on Rails member

@gazay no, I don't think we want a patch for master for this. People should probably just blow away their cache if they're going from Rails 3.1 all the way to 4.0.

@tenderlove
Ruby on Rails member

@gazay I've reverted this pull request as it broke the tests in actionpack. Can you please make sure the tests pass and send another pull request? Thanks.

@tenderlove tenderlove added a commit that referenced this pull request Dec 29, 2011
@tenderlove tenderlove Revert "Merge pull request #4196 from gazay/3-2-stable-marshalling"
This reverts commit df37018, reversing
changes made to a0fd9fb.
80f2eeb
@mech

What happen if I have 2 Rails app in the same machine sharing 1 memcached server. I have Rails 3.0.7 and Rails 3.2.0 application running on same machine and they both need to use a single memcached server. In this way, I always have error because cache written by 3.2.0 was read by 3.0.7 as and not an object :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment