This project is archived and is in readonly mode.

#785 ✓duplicate
Morten Christensen

Caching models fails in development

Reported by Morten Christensen | August 8th, 2008 @ 06:44 PM | in 2.x

The new Rails.cache.fetch functionality in Rails 2.1 fails when caching ActiveRecord models on the second request in development mode (apparently caused by reloading classes). This problem has been reported by many such as f.x:

http://blog.bashme.org/2008/07/2... http://jeronrails.blogspot.com/2...

I have also run into this critical problem myself. This should work without problems according to the docs and other community info such as this "http://railscasts.com/episodes/115" which explicitly says that caching of models works (which is does not) as this bug report says.

Comments and changes to this ticket

  • Ryan Bates

    Ryan Bates August 8th, 2008 @ 08:10 PM

    • Tag changed from “cache” to “bug, cache”

    I have not tested it yet, but I think this can be fixed by changing how MemoryStore works. Right now it just stores the object itself in memory. This becomes a problem when Rails reloads the classes in development. It also has nasty side effects like storing associated objects if they have been loaded.

    Instead I would like to see MemoryStore run the object through Marshal just like MemCacheStore does.

    Also (probably should be another ticket) AFAIK it's not possible to store regular objects in FileStore, only strings. It would be nice if Marshal was used here too. This way we can get consistent behavior across all cache stores.

  • Ryan Bates

    Ryan Bates August 8th, 2008 @ 09:45 PM

    Here's a simple patch to get Marshal in MemoryStore. This appears to solve the immediate problem of using it in development mode.

    However, I do not know if this is a good idea. Taking a look at the source, it appears only CompressedMemCacheStore uses Marshal (I was mistaken above). I wonder what the overhead is.

  • Ryan Bates

    Ryan Bates August 8th, 2008 @ 10:01 PM

    Sorry to keep responding to myself, but I just realized my original assumption was correct. MemCacheStore does use Marshal internally unless the :raw option is passed.

    I'm all for MemoryStore using Marshal now.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 12:00 AM

    Okay, I think I found a good solution. Attached is a patch which uses Marshal in both MemoryStore and FileStore. Both accept the :raw option allowing you to bypass the Marshal just like MemCacheStore does.

    I altered fragment caching to use the :raw option so it stores the plain string in the file to make it compatible with previous versions.

    It still needs some more testing, but I believe this resolves all of the issues mentioned above. Caching with MemoryStore now works in development, and FileStore accepts other objects besides strings. This also makes the cache stores behave consistently so you don't get odd behavior when switching from one to another.

  • Jacek Becela

    Jacek Becela August 9th, 2008 @ 12:16 AM

    Elegant solution. +1

  • Jack Danger

    Jack Danger August 9th, 2008 @ 05:16 AM

    Love it. +1

  • josh

    josh August 9th, 2008 @ 06:40 AM

    • Assigned user set to “josh”
  • Morten Christensen

    Morten Christensen August 9th, 2008 @ 11:35 AM

    Won't the changes using Marshall make caching work MUCH slower in production ?

    Maybe the Marshall trick should only be used when in development mode (or better when reloading of classes is enabled) ?

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 04:41 PM

    • Tag changed from “bug, cache” to “bug, cache, patch”

    I haven't done any benchmarking yet, but from my understanding Marshal is very fast. Just the fact that memcache uses it internally should say something.

    YAML is another alternative, but that is slower than marshal.

    True we could just enable it in development when classes aren't cached, but there's a lot of side effects to the old way of doing things. For example, loaded associations will stay in memory and fill it up pretty quickly. My concern is developers will test the cache behavior in development and get to production where bad things happen. I'd prefer it behave the same way in both.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 04:43 PM

    One more thing I forgot to mention. If Marshal is the bottleneck in your performance tests, you can always pass the :raw option and bypass it on a case-by-case basis.

  • josh

    josh August 9th, 2008 @ 04:47 PM

    • State changed from “new” to “wontfix”
    • Tag changed from “bug, cache, patch” to “bug, cache”

    Yeah, marshaling is going slow down writes and more important, slow down reads. I don't really think you need to be caching in dev mode anyway ;)

    memcache already marshals since it only supports strings. Filestore should probably marshal too, but it doesn't really make sense for memorystore though. Objects will always read and write fine from it (except in dev mode ;)

    Show me the benchmarks and we'll see if it really makes a difference.

    Possibly an exception for dev mode for memory cache is in order.

  • josh

    josh August 9th, 2008 @ 04:47 PM

    • State changed from “wontfix” to “open”

    (leave it open, one of you nice guys is going fix it)

  • Morten Christensen

    Morten Christensen August 9th, 2008 @ 05:13 PM

    Not sure I understand the comment "I don't really think you need to be caching in dev mode anyway ;)"....... The need (or no need) in dev mode is not relevant. What is important that the exact same code that I have in my Rails projects works in both dev mode and production.... However fixing dev mode should at best not degrade production performance of cause.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 05:20 PM

    From my understanding, the point of Rails.cache is to provide a consistent interface for caching where you can swap out the store with ease. If the stores all behave drastically different (as far as what you can store in them), then doesn't this defeat the purpose?

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 05:25 PM

    Some quick and dirty micro-benchmarking: http://gist.github.com/4683

    Basically, marshaling is 20x slower to save and 100x slower to load.

    However, Marshalling 100,000 simple AR objects still takes only 10 seconds or so, so despite how bad 100x sounds it might not make much difference in an actual real world requests. That's milliseconds of differences - and still magnitudes quicker then reloading from the db.

    0.00005 vs 0.000005, who cares?

    If it helps with consistency, marshaling is a good idea.

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 05:27 PM

    Oh wait, my math was based off reloading strings. It's worse then I thought in my previous comment, sorry.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 05:41 PM

    Thanks for doing that Jim. If we take out the Student object then it's roughly 10x slower, is that correct? It seems Marshal has trouble with custom Active Record objects.

    Currently we can't really store Active Record objects in Memory Store without bad side effects so slow speed is better than nothing. :)

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 05:55 PM

    Yeah, about an order of magnitude.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 06:02 PM

    Also, for the database reload, it's best to disable ActiveRecord caching so it hits the database each time. Here's my numbers for that.

    http://gist.github.com/4688

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 06:33 PM

    Okay, in summary. Using Marshal for MemoryStore is about 10x slower for simple objects. Josh, does this seem acceptable?

    Complex objects (such as ActiveRecord models) may be 100x slower, but that will vary wildly depending on the object's complexity. It is still 23x faster than fetching from the database in my rough test.

    However, as mentioned above, MemoryStore's current behavior does not lend itself well to complex objects anyway. This is because changing the state of the object after caching will change the cache itself. This does not seem right to me.

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 09:26 PM

    Maybe dup objects before storing them, and then dup them on retrieval and return the copies to prevent tainting the cache?

    Dup is even faster then marshal.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 11:01 PM

    • Tag changed from “bug, cache” to “bug, cache, patch”

    That's an excellent idea Jim! Attached is a patch to do this in MemoryStore instead of Marshal. This seems to solve the development issue as well. And as you said, dup is much faster.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 11:07 PM

    Oops, I take that back. dup doesn't solve the development issue. It still results in the same "stack level too deep" error.

    Should we just disable this kind of caching by default in development? I'm thinking a "NoStore" cache store which just returns nil on read. What do you think?

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 11:08 PM

    One thing your patch is missing (the only thing, everything else looks good) is a rescue TypeError when dup is attempted on inputs like false or nil.

    Maybe something like this?

    def dup_value(value) value.dup rescue TypeError value end

    ... and then

    @data[name] = dup_value(value.dup)

    instead of

    @data[name] = value.dup unless value.nil?

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 11:09 PM

    Oh, shoot. Yeah, the class gets nuked out from under it still.

  • Jim Lindley

    Jim Lindley August 9th, 2008 @ 11:10 PM

    Dup'ing MemoryStore still a good idea in production, though.

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 11:24 PM

    Updating patch with Jim's suggestion.

  • José Valim

    José Valim August 9th, 2008 @ 11:31 PM

    I agree with the consistent interface and I also would like to see Marshalling as option in other stores.

    We can use Ryan Bates patch, but on MemoryStore and FileStore the :raw option is set as default to true. So no Marshalling would be done in existent apps, no performance lost (except that this would be different from Memcached default).

    Another thing is that on development environment, all objects should be marshalled, ignoring :raw option. We can create a config variable that would be set in the development environment:

    config.always_marshal_objects_on_cache = true

    How does it sound?

  • Ryan Bates

    Ryan Bates August 9th, 2008 @ 11:41 PM

    José, several good ideas there. If we supply options for changing the default behavior on memory store cache then we can leave it up to the developer to choose depending on the environment.

    I think FileStore should marshal by default though. you can't really store anything in there except strings without Marshal.

    As for the option passing, I think they should be specified when choosing the store. For example:

    
config.cache_store = :memory_store, { :raw => true }

    This way we don't have a global option which only effects one store.

  • Ryan Bates

    Ryan Bates August 10th, 2008 @ 02:01 AM

    • no changes were found...
  • Ryan Bates

    Ryan Bates August 10th, 2008 @ 02:01 AM

    Here's another patch. This one allows you to set the :raw option while setting the memory store. You can also pass the :raw option during read/write.

    
config.cache_store = :memory_store, { :raw => false }
# or
Rails.cache.write('foo', 'bar', :raw => true)
Rails.cache.read('foo', :raw => true)

    Like the other stores, setting raw will bypass Marshal. The primary difference here is that raw defaults to "true" if it's not specified as that's usually the behavior one would want. "dup" is used for raw.

    In development we want to use Marshal, in production we don't. This patch handles this automatically by setting the default cache store depending on the current environment.

    
# in initializer.rb
def default_cache_store
  [:memory_store, {:raw => (environment != 'development')}]
end

    This method was defined before, but apparently it was never called so I assume it's okay to overwrite it with this.

    What do you think? Any questions?

  • José Valim

    José Valim August 10th, 2008 @ 08:52 AM

    Awesome! Ryan, in your patch, are we able to set the defaults in other Stores also? In other words, are we able to do:

    
  config.cache_store = :file_store, { :raw => true }

    Looks like we don't... I think would be nice if we provide the same interface to all Stores, no?

  • Ryan Bates

    Ryan Bates August 10th, 2008 @ 04:33 PM

    I considered doing that, but I decided not to. I don't see any reason why one would want to because it makes the stores behave very differently. If you do this with file or memcache store that basically means you can only store strings.

  • José Valim

    José Valim August 11th, 2008 @ 12:53 AM

    I was thinking more in providing an option for those who are using FileStore currently (this way they won't have to append :raw => true in all cache calls after this patch is accepted).

    But investigating further, if they are caching strings, they are probably using the method cache defined in:

    action_view/helpers/cache_helper.rb

    Since this methods caches only strings, would be interesting if the options :raw => true is set as default in this helper. Otherwise a lot of application will start to Marshal unnecessary.

    What do you think? =)

  • Ryan Bates

    Ryan Bates August 11th, 2008 @ 01:16 AM

    The "cache" helper method uses fragment caching, and I set :raw => true for all fragment caching. This way it behaves like it used to and will be compatible with existing caches.

  • José Valim

    José Valim August 11th, 2008 @ 08:24 AM

    Amazing! +10 =)

  • josh

    josh August 11th, 2008 @ 01:58 PM

    • Tag changed from “bug, cache, patch” to “cache, patch”

    Also, the :raw option is the wrong API. The only reason it is used is for the memcache-client. A separate class would be nicer.

    
module ActiveSupport
  module Cache
    class MarshaledMemoryStore < Store
      def read(name, options = nil)
        super
        Marshal.load(@data[name])
      end

      def write(name, value, options = nil)
        super
        @data[name] = Marshal.dump(obj)
      end
    end
  end
end

    However, it still think we are chasing the wrong problem. There is nothing wrong with the current ActiveSupport::Cache::MemoryStore, the issue is class reloading. I still agree with marshaling FileStore, but thats not what we are talking about here.

    I really think you should NOT be using caching during "development" mode. And if you goal is to get the same "experience" as your "production" mode, consider turning cache_classes on or create a separate environment called "development_with_caching" and work on that when you need to test your caching.

  • Ryan Bates

    Ryan Bates August 11th, 2008 @ 03:07 PM

    Josh, I think the existing MemoryStore implementation is faulty because changes made to an object afterwards effect the cache. This happens in production as well.

    
cache = ActiveSupport::Cache.lookup_store(:memory_store)
h = {:a => 1}
cache.write('foo', h)
h[:a] = 2
cache.read('foo')[:a] # => 2

    I don't know of any other caching technique that behaves this way, and I think it's one of the core problems with MemoryStore itself. This problem doesn't really have anything to do with the original ticket, but I was trying to tackle both at the same time since they are similar. Perhaps I should post this in a separate ticket?

  • Jim Lindley

    Jim Lindley August 11th, 2008 @ 03:47 PM

    If the cache contents can be altered by changing fetched objects, then it's not a cache store, it's a fancy global variable to store objects in.

  • Jim Lindley

    Jim Lindley August 11th, 2008 @ 04:00 PM

    New ticket #799 created to move the MemoryStore issue to, to not detract from the class reloading in development problems.

  • Ryan Bates

    Ryan Bates August 11th, 2008 @ 04:55 PM

    I also moved the FileStore issue into a separate ticket (#800) as that is fairly distinct.

    Now we can focus on solving this development environment issue on its own. Josh, one idea I had is to make a "NoStore" cache store which just returns nil on read. This would effectively turn off caching in development mode. Does this sound reasonable? If so I'll make a patch.

  • josh

    josh August 11th, 2008 @ 09:09 PM

    @Ryan good point about the mutable contents. I think all cached objects oughta be immutable.

    I also think a NoStore option would be a good workaround. I would actually expect the MemoryStore to be erased on each request because of reloading. However its setup in the framework which is not reloaded. I don't think its wise to count on the persistence of any memory store or cache with cache_classes on.

    I also don't think any consistent behavior is lost either. It is really great that you can test caching locally without setting up memcache on your own machine. The MemoryStore is an awesome MockMemcacheStore.

  • Ryan Bates

    Ryan Bates August 11th, 2008 @ 11:29 PM

    Here's a patch that adds NoStore which always returns nil on read. This also sets NoStore as the default store in development. This will, in a sense, disable caching in that environment.

  • Rob Anderton

    Rob Anderton August 12th, 2008 @ 08:28 AM

    Given that both the memcache and file stores (thanks to Ryan's other patch) will marshal objects, why is it such a horrible idea to use marshalling on the memory store? It's not so awesome a MockMemCacheStore if it doesn't work like it in development mode.

    The NoStore option is nice to have if a developer specifically wants to turn off caching (although personally I'd have called it NullStore, but that's just me!) but I don't see why I should need to create a new environment with specific settings just so I can use the memory store in development mode - it should just work.

  • jshumate

    jshumate August 14th, 2008 @ 01:31 AM

    I have tested the attached patch and still have problems with methods disappearing from models on the second request. Running in production mode works fine. Still not convinced that anyone has gotten to the bottom of the real problem.

  • josh

    josh August 14th, 2008 @ 01:33 AM

    @jshumate Still not convinced that anyone has gotten to the bottom of the real problem.

    Agreed.

  • josh

    josh August 14th, 2008 @ 01:36 AM

    I'm now even having second thoughts about marshaling filestore. Maybe we should just say RAILS_CACHE is a simple cache store for strings only.

  • Ryan Bates

    Ryan Bates August 14th, 2008 @ 04:12 AM

    Caching models with memcache is a pretty powerful technique, and I've heard several say they've had great success with it. Since there is a memcache store, it seems like Rails.cache should support it. And, AFAIK, it already does in that specific store.

    The question is, should all caching stores behave the same way?

  • jshumate

    jshumate August 14th, 2008 @ 08:26 AM

    I think there are several deep questions regarding caching, but I thought they had moved to their own thread. This thread started with a report of behavior that I have been fighting for several weeks now without finding the root cause. Since 2.1 classes will lose methods after the second request in development mode. No problems in production and no problem on the initial server startup. Usually you just get a method missing error, but I also had it lose the id method and infinite loop in method missing as a result. Seems to be more likely in models with has_many :through, but I may have seen it elsewhere and just don't remember.

  • Ryan Bates

    Ryan Bates August 14th, 2008 @ 03:08 PM

    jshumate, have you been experiencing this same error through other means besides caching?

  • jshumate

    jshumate August 14th, 2008 @ 04:34 PM

    I'm not sure exactly how to answer that. I am using edge with only a handful of plugins, all of them were disabled in my early tests to see where these seemingly random errors were coming from. So any caching that is enabled by default in the development configuration is active, but I am making no specific use of any caching mechanism.

    It appears to me that that what is happening is that when the code is re-loaded in development mode something happens to some of the methods in the model. The only database backed accessor that I have seen go missing is id, but many of the non column based methods in my models will disappear. In the debugger I can look at the object that threw the error and the methods aren't there, but if I instantiate a new object of the same class while at the breakpoint they are all present.

    So I can see the possibility that this is an interaction with some of the caching code, but it could also just be an error in the code that does the reload on each request.

  • Tom Lea

    Tom Lea August 14th, 2008 @ 05:48 PM

    • no changes were found...
  • Tom Lea

    Tom Lea August 14th, 2008 @ 05:48 PM

    Right, managed to replicate what I think jshumate was experiencing.

    The attached patch seems to fix it for my simple test site (sorry no unit tests)... but if I'm honest, I'm not sure why.

    It was based on the assumption that the generated code was vanishing on reload (which I can't replicate out side of AR), and because the function that generated it (#id) was being clobbered by the generated code (also #id), it would never be recreated again.

    I still can't replicate this with none AR classes, which makes me think my solution could be right, but my reasoning is almost definitely wrong.

  • jshumate

    jshumate August 14th, 2008 @ 05:59 PM

    Looking at the patch from Tom Lea it would probably fix the stack level to deep issue when id goes missing, but it would not address the method missing issue for other methods. The problem seems deeper than what this patch addresses.

  • Ryan Bates

    Ryan Bates August 14th, 2008 @ 06:12 PM

    Did you just start experiencing it in 2.1? Might be worth investigating what changed between 2.0-2.1 as far as reloading goes.

  • Rob Anderton

    Rob Anderton August 14th, 2008 @ 07:19 PM

    We seem to be moving onto a different issue and one I've always assumed was expected behaviour. I'd thought that by reloading a class' definition you were effectively kicking its legs out from beneath it - and it came as no surprise that it would then fall over.

    Here's an example from Rails 2.0.1:

    
u = User.find(1)

u.class == User
=> true

u.id
=> 1

# Age is a custom method rather than a database attribute
u.age
=> 10

reload!

u.class == User
=> false

u.id
=> 1

u.age
=> NoMethodError: undefined method `age' for #<User:0x6787048>
  from ../vendor/rails/activerecord/lib/active_record/attribute_methods.rb:205:in `method_missing'

# Load a fresh one
u2 = User.find(1)

u2.class == User
=> true

u2.id
=> 1

u2.age
=> 10

    The same thing happens if we go way back to Rails 1.2.5 (although the NoMethodError is in method_missing in base.rb line 1860).

    Obviously recent changes to the reloading code have meant this behaviour has changed slightly in 2.1 (the stack error is a new one), but the underlying problem has been there for some time.

    ...Meanwhile back at the original ticket...

    To answer Ryan's earlier question: I think the various cache stores should provide provide the same interfaces without needing specific workarounds - I should be able to switch from MemoryStore to FileStore to MemCacheStore without changing anything other than a configuration setting. And I should be able to use any of them in any environment (development, production, testing, or any other).

  • Tom Lea

    Tom Lea August 14th, 2008 @ 08:21 PM

    Rob is right about consistency... but we need core to agree on what to consist upon (if you follow my bad grammar).

    Should Cache::Store just store strings, or should they serialize other objects?

    @jshumate: Which other methods go missing? Attribute readers seem to work OK for me on edge. #id is a particularly special case after all. We need to find a consistent common factor.

    @Rob: I think it could be more complex/subtle than this, as none AR stuff does not seem to show the same behaviour (see http://pastie.org/253127 for a rubbish example of it not being like that.) The same seemed to apply to edge on my other machine.

  • jshumate

    jshumate August 15th, 2008 @ 01:46 AM

    @Rob Anderton Original Ticket -- The new Rails.cache.fetch functionality in Rails 2.1 fails when caching ActiveRecord models on the second request in development mode (apparently caused by reloading classes).

    I remain unconvinced that this is necessarily a caching issue. I believe this ticket is about reloading classes. The cache discussions seemed to have been moved to thread 799.

    As I said above I have yet to see a method backed by a database column go missing with the exception of id which is a special case anyway. Other methods in the class go missing without much rhyme or reason. Again as I said above when I break before the exception is raised and look at the object the method that is about to be tagged as missing isn't there and usually a few others are missing as well. If I instantiate a new object in the debugger all the methods are there. This only happens after the second request and I don't see it in production. Not sure how far back it goes, but I only noticed it after the 2.1 milestone. I'm using some 2.1 features so it is a bit hard to regress, but I will see what I can do.

  • Rob Anderton

    Rob Anderton August 15th, 2008 @ 09:46 AM

    I agree it's not a caching issue - it is a class reloading issue and as I showed in my last post it goes back at least as far as Rails 1.2.5.

    To quote a Giles Bowkett blog from last year:

    It's probably possible to write some Ruby which queries ObjectSpace for ActiveRecord models, loads their attributes off into a hash, and then reinstantiates them - but making something like that portable could take some serious wizardry.

  • Tom Lea

    Tom Lea August 15th, 2008 @ 10:46 AM

    @Rob: It seems Giles is referring to the opposite effect. Ruby objects get disjointed from their classes on initialization.

    This has been true for a long time and is due to the constant being removed before the class is defined. This means that both classes exist at the same time, both having the same name, but the first one no longer having anything to do with the constant that referenced it.

    Code to show my point: http://pastie.org/253468

  • Tom Lea

    Tom Lea August 15th, 2008 @ 11:17 AM

    s/Ruby objects get disjointed from their classes on initialization./Ruby objects get disjointed from the constant that references their class on initialization./

  • Ryan Bates

    Ryan Bates August 15th, 2008 @ 03:19 PM

    I agree that the core problem is not a caching issue. It will show up whenever you're storing a model globally between requests. However, caching seems to be the only place where the framework directly supports storing a model globally (unless we say it should only store strings).

    If we can solve the core problem of reloading models, that's awesome and would be preferred. But it sounds like this is the way Rails has always behaved, and a solution may require some serious changing of how reloading works in Rails (I could be wrong about that).

    Because that is such a big problem, I think we should take it one case at a time. If we can solve this caching issue in development, then I would consider this ticket resolved. Then we can open up other tickets about other cases where we store models globally.

  • jshumate

    jshumate August 15th, 2008 @ 04:38 PM

    I am not storing models globally.

    The problem is that when the development environment reloads the code on a new request the AR models are getting clobbered in some way. They work fine on startup and sometimes it takes a few requests to hit a method that has been clobbered, but after that you are dead until you restart the server.

    I've seen the issue with reload! in the debugger forever which is why I always restart the console after code changes. What I am seeing now is new to me and I don't think it has always been that way. I think we have a lot of people commenting on this thread about similar behavior and issues, but based on my read of the original post this thread is about problems in development mode with reloading code.

    If I'm wrong about that and others are only seeing problems related to caching after a reload then maybe we can start a new thread specifically about issues with AR classes being corrupted in development mode when they are reloaded.

  • jshumate

    jshumate August 15th, 2008 @ 04:45 PM

    Looking back at Rob Anderton's post from the console this is not the same thing.

    I am using the debugger and looking at models that are created during the request cycle after the reload has occurred. Not models that existed before the reload.

    This is just a fairly normal application. I get a method missing exception and restart the server then hit refresh in the browser and it works fine. Hit refresh again and it gets a method missing error. This is repeatable. If I break in the code after the find returns a model on the first try after server startup all the methods are there after the reload they aren't.

    Run the same code in production mode no issues at all.

  • Ryan Bates

    Ryan Bates August 15th, 2008 @ 04:59 PM

    @jshumate, sorry for the incorrect assumption. I haven't seen this behavior outside of storing a model globally. Could you post your code or example app somewhere which has this issue?

  • Tom Lea

    Tom Lea August 15th, 2008 @ 05:18 PM

    • no changes were found...
  • Tom Lea

    Tom Lea August 15th, 2008 @ 05:18 PM

    • Tag changed from “cache, patch” to “activerecord, bug, dependencies, patch”

    New lead: It seems that all inheritable_attributes are going missing... but only on AR objects. Using inheritable_attributes on none AR objects works fine after a reload.

    The attached patch seems to fix the issues I am seeing by removing the functionality that removes methods and variables from AR Classes.

    The strange thing is that it does not seem to break anything else... does anyone know what it's supposed to be achieving? As far as I can tell, it resets class variables on every class inherited from AR::Base, but it removes the constant anyway.

    Could this be a hangover from Reloadable, looks like the logic was added way back in 2006 (http://github.com/rails/rails/co...>

    I don't suppose technoweenie could shed any light on this?

  • jshumate

    jshumate August 15th, 2008 @ 05:26 PM

    • Tag changed from “activerecord, bug, dependencies, patch” to “cache, patch”

    @Tom Lea

    wow! good catch, great question.

  • jshumate

    jshumate August 15th, 2008 @ 11:56 PM

    I have had a chance to run the app for a bit in development mode with the 2 lines from the last patch commented out and can't find anything broken.

    I also am unsure why this code is there. It is only called from one location with the comment "that its cleaning up the application by clearing out loaded classes so that they can be reloaded on the next request"

  • Ryan Bates

    Ryan Bates August 16th, 2008 @ 12:05 AM

    Is it possible the two lines were intended to solve a memory leak? That may explain why it doesn't make a noticeable change in behavior. It's a bit beyond me, but just a thought.

  • jshumate

    jshumate August 16th, 2008 @ 12:19 AM

    I'm not sure why you would only need to clean out AR and not other classes. Someone clearly had something in mind, but the comment and usage don't make it very clear what it was.

    Of course there is the flipside of this which is why aren't the methods reloaded since clearly this code assumes they will be?

  • jshumate

    jshumate August 16th, 2008 @ 12:31 AM

    I paused for a minute to think about what I just said and realized that I may have hit on why this just turned up. If the problem isn't that the methods are deleted, but that they are no longer loaded properly then that would explain why this surfaced now. Looking for changes to the code that brings them back in is starting to sound like a good idea.

  • Ryan Bates

    Ryan Bates August 16th, 2008 @ 12:58 AM

    After a little bit of research I found this commit which appears to be when those two lines were added.

    As suspected, this was intended to solve a memory leak. But considering it was almost 3 years ago, I'm not sure if it's still applicable. I'm also wondering why the test didn't trigger?

  • Tom Lea

    Tom Lea August 16th, 2008 @ 03:44 PM

    I'm wondering if this fixed the memleek issue and not the initial adding of those two lines... I'll try and do more investigation when I get time.

  • jshumate

    jshumate August 16th, 2008 @ 06:08 PM

    I'm still thinking that this hasn't been a problem for 2+ years and nobody noticed it. I'm trying to find anything recent that changed the way AR reloads that would explain why this just showed up in my projects in the last few months. For all I know the patched lines are how it should be and the error is in on reload.

  • jshumate

    jshumate August 16th, 2008 @ 07:16 PM

    Having spent a bit of time debugging after the last patch install I no longer get any method missing errors, but changes to the code are not getting picked up consistently and I am having to restart the server to see them. So I am now completely convinced that this bug is not in the code that removes methods, but somewhere in the code that puts them back and I'm guessing it will turn up in a recent change.

  • Tom Lea

    Tom Lea August 16th, 2008 @ 10:33 PM

    @jshumate are you failing to see changes to cached objects, or new instances?

    Cached objects will keep a reference to their original class and methods, so changes only apply to new instances. This is intended behaviour for Ruby.

  • jshumate

    jshumate August 17th, 2008 @ 03:59 AM

    To be honest I haven't dug deeply, but several times I have had an error that didn't make any sense because the debugger state did not reflect the current code. I restarted the server and all was well. It seemed as if changes I was making to the source were not being reflected and if the changes affected a call to code that was not in sync I got an error.

    Feels like the previous behavior in reverse. Instead of having methods go missing now they don't seem to all be be most recent versions so they can conflict. This reinforces for me that we still aren't to the bottom of this. Something that I haven't found yet changed in how AR reloads during development.

  • Tom Lea

    Tom Lea August 17th, 2008 @ 11:12 PM

    • Tag changed from “cache, patch” to “activerecord, activesupport, bug, cache, patch”

    @jshumate this basically comes down to you caching the old object with it's old methods and old behaviour. As far as I am aware, there is no magic to prevent this.

    If you cache a serialized version of your models rather than the actual models the behaviour will be updated OK. This is because the deserialization will create a new instance and load up the attributes of the new class definition.

    Caching a none serialized instance will keep it exactly as it was. So no changes to methods in the new base class will be applied.

    My suspicion as to the cause of the reported change of behaviour could be related to an unexpected change of caching mechanisms, moving from either mem_cache_store, compressed_mem_cache_store, file_store or drb_store to memory_store. This could be triggered by the tmp/cache dir being deleted in default configuration (IIRC).

    As part of a separate patch I think memory_store should start serializing by default (at least in development). But even if this does happen, it would be nice to be able to be able to do so in raw form, just for when performance really matters.

  • jshumate

    jshumate August 18th, 2008 @ 01:55 AM

    • Tag changed from “activerecord, activesupport, bug, cache, patch” to “cache, patch”

    Not sure how many times I have to go through this. I am not caching anything between requests. Each request begins with a find of the objects and I've watched the log and seen a new request to the db. Without that last patch models have methods missing, with the last patch methods do not get updated while making code changes without restarting the server.

    In production mode everything works as normal. I just don't see how to be more clear and the patch in question has nothing to do with caching it only prevents methods from being removed from AR classes clearly depending on the reload to put them back, but it doesn't put them all back. Something is different in the code that loads classes on server startup and the code that reloads classes on each request. I don't see how that is caching related.

  • Ryan Bates

    Ryan Bates August 18th, 2008 @ 02:56 AM

    jshumate, do you have an example app or piece of code that replicates your problem? I think that would help us debug it.

  • Tom Lea

    Tom Lea August 18th, 2008 @ 02:14 PM

    @Ryan, figured out why there was no sign of the test failing, looks like it was removed. I tried to reconstruct the test again using updated interfaces, but I can't get it to pass without my patch, never mind with.

    @jsumate, if there is no caching happening we need to see code... the only replication I can do is either using MemoryCache, which this should fix, or keeping instances in global variables ($fish ||= Fish.new; $fish.name).

  • Steven Soroka

    Steven Soroka August 20th, 2008 @ 03:46 AM

    This has been bugging me since I starting using Rails.cache, so that's my only frame of reference for it here. I'm not sure I follow what exactly the problem is, but I have made a very small console snippet that reproduces my problem, here:

    http://pastie.org/256177

    I use reload! to kind of wipe the slate clean and still grab the next request from the cache. the cache appears to return the object, but simple requests to it fail, like .id

    I'm kinda lost here but would love it fixed. :)

  • josh

    josh August 20th, 2008 @ 05:20 PM

    • State changed from “open” to “hold”
  • namxam

    namxam September 11th, 2008 @ 05:39 PM

    I am seeing the exact same problem. Actually it poped up in my code when i was including the social_feed plugin. But as nobody using this plugin has this problem, it seems to be related to my specific setup. Nevertheless, when accessing the page in development mode, the first request works and as soon as I reload the page everything is crashing with a "stack level to deep" error. Turning on model caching in development mode resolves the issue (as well as changing to production mode). The stack level too deep error seems to be related to not existing methods and properties. But I haven't had time to go deeper and find the root of the problem.

  • Glenn Powell

    Glenn Powell September 12th, 2008 @ 08:03 PM

    I actually see a "similar" issue now. I don't get crashes, but I do have method-not-found errors upon reload in development. I have a model "Language" with a defined method "name". Everything works fine for the first view, but on page reload, the methods "name" and "id" are stripped from the Language model.

    Also class caching seems to be always turned on, even in development mode (with the config.cache_classes set to false).

    Anyone else seeing something like this?

  • Mario Uher

    Mario Uher September 15th, 2008 @ 09:17 AM

    Same problem, solved by adding this:

    @@@ruby module ActiveSupport module Cache

    class BlackholeStore < Store
  def read(name, options = nil)
    super
    nil
  end

  def write(name, value, options = nil)
    super
  end
end

    end end
  • Steven Soroka

    Steven Soroka September 15th, 2008 @ 10:51 PM

    @Glenn: Yeah, that's it exactly, my cached objects are really screwy.

    I've looked in to this and it seems to me that the problem is that Marshal.load() (which is called by the cache) isn't checking to see if the object it's creating is properly autoloaded yet.

    Perhaps the answer lies in a smarter cache, fixed marshal, or in ActiveSupport::Dependencies somewhere. I dunno.

  • Glenn Powell

    Glenn Powell September 16th, 2008 @ 11:59 AM

    • Tag changed from “cache, patch” to “cache, patch”

    Well, actually I tracked my particular problem down to a dependency issue. I was requiring that one model class in another custom class using require instead of require_dependency.

    But my (erroneous) code was in fact working not too long ago, so I think something definitely changed with respect to class loading/caching.

  • Steven Soroka

    Steven Soroka September 17th, 2008 @ 04:18 PM

    I've managed to patch the Marshal.load problem here, and it seems to resolve the problem. I'll look in to it deeper but I thought some of you might want to try it. Throw this in a file in your auto_loader.rb or whatever.

    module Marshal class <<self

    def load_with_preload_class(str, &p)
  str.scan(/.o:.(\w+)/).each{|matches|
    begin
      # by constantizing the class name,
      # it should trigger it to be autoloaded.
      matches.first.constantize
    rescue NameError
      # I guess that wasn't a class we found after all.
      # Nothing to see here, move along
    end
  }
  load_without_preload_class(str, &p)
end
alias_method_chain :load, :preload_class

    end end

  • Steven Soroka

    Steven Soroka September 17th, 2008 @ 04:29 PM

    Oops, I missed namespaced models. Use this instead:

    module Marshal class <<self

    def load_with_preload_class(str, &p)
  str.scan(/.o:.([\w:]+)/).each{|matches|
    begin
      # by constantizing the class name,
      # it should trigger it to be autoloaded.
      matches.first.constantize
    rescue NameError
      # I guess that wasn't a class we found after all.
      # Nothing to see here, move along
    end
  }
  load_without_preload_class(str, &p)
end
alias_method_chain :load, :preload_class

    end end

  • Chris Gansen

    Chris Gansen October 2nd, 2008 @ 08:05 PM

    I'm running into the same issue @jshumate is witnessing: in development environment AR objects will "lose" methods on the second (and all subsequent) request. I've disabled all manner of caching, to no effect, so I doubt that's it. What's notable is that the missing method, a belongs_to association, is defined in an included module, not the AR object itself.

    Enabling class caching via

    
config.cache_classes = true

    helps, but doesn't address the issue with reloading.

  • Geoff Buesing

    Geoff Buesing November 5th, 2008 @ 03:35 PM

    Looks like we're discussing the same issue in this ticket:

    http://rails.lighthouseapp.com/p...

    David Rice has pulled together an example app that shows the issue: http://github.com/davidjrice/bug... (he's caching an AR object between requests in a class variable.)

    As far as I can tell, this issue is present in 2.1.x as well as edge.

  • josh

    josh November 10th, 2008 @ 06:40 PM

    • State changed from “hold” to “duplicate”

    See #1339 for the short story

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Tags

Referenced by

Pages