Running out of memory due to code reloading feature in v4.0.6

[rwadstein]

4.0.6 introduced the ability to reload the app in an attempt to behave like a Rails web application when the configuration cache_classes == false.

Unfortunately, with our very large application, the worker which executes roughly every 5 seconds keeps reloading the app and memory usage increases pretty quickly.

The memory footprint of of our background rake task grew from about 300MB to 1.5G in about an hour when the cache_classes = false and no jobs were even being processed. Setting it to true alleviates this problem, but it affects our daily dev environment.

[albus522]

We are doing exactly what rails server does in dev mode.

Can you produce an example app that reproduces the issue?

[rwadstein]

I can try to create a sample app that can represent our environment. It is possible that even the mechanism that a rails server reloads may have the same problem for us. It may just not be so obvious since we don't reload so many times as opposed to when the delayed job worker does it every 5 seconds. I will get back to you as soon as a I can.

[rwadstein]

I managed to find some more detailed info as to why this is happening. It appears we have had this symptom for a long time, but it never got visibly exposed until delayed job started reloading. This obviously would only exist in a dev environment since that is the only environment where we do not cache classes.

Our application has lots of engines that have initializers in the engine.rb file that looks similar to this:

initializer 'blorgh.initializer' do
  ActionDispatch::Reloader.to_prepare do
    require 'blorgh/mixins/enhance'
    Blorgh::Article.send(:include, Blorgh::Mixins::Enhance)
  end
end

With this, I could easily detect when our mixins are being reinserted into the classes.

So despite the fact that you are doing exactly what the rails server does in dev mode, Rails will not reload until something has changed. Unfortunately, with the delayed job reloader, it doesn't care if something has changed or not, but it will call reload every 5 seconds when the worker is launched. Because we have Reloader do blocks, it executes them regardless and causes more memory to be used by reinserting all our mixins. In our case, the increase is significant every reload.

I created a very simple application with one engine that has a reloader block like above and 10 models it is including the mixin into. Each delayed job reload caused the memory to go up by 1MB. The same was true for triggering a Rails reload as well, but I had to make a model change first in order to do so, otherwise each refresh on the page was fine.

If appears that there needs to be some sort of additional check before reloading similar to how rails determines to reload.

[albus522]

Rails does not detect changes. It reloads for every request. The difference is if you aren't doing anything on the server it never reloads.

I suggest you change that to

initializer 'blorgh.initializer' do
  ActionDispatch::Reloader.to_prepare do
    require 'blorgh/mixins/enhance'
    Blorgh::Article.send(:include, Blorgh::Mixins::Enhance) unless Blorgh::Article.included_modules.include?(Blorgh::Mixins::Enhance)
  end
end

[rwadstein]

Maybe the difference is with the different versions of Rails. We use 3.2.21.
Specifically in 3.2.21, I managed to debug Rails and found it would validate whether it needs to reload based on the the evaluation of reload_dependencies?

This condition was called by the reloader middleware here and was used to determine if the reloader callbacks should be executed or not.

This would change between true and false depending on whether or not something changed and would reload accordingly.

[albus522]

It looks that that has moved around a lot. I'll take a look at what we might be able to do for conditional reloading but in the meantime my suggestion stands.

[rwadstein]

Thank you and I greatly appreciate your input. I definitely agree we should not include modules if they are already included, so I will discuss with our platform team and the other devs to possibly include checking modules as you suggested.

For the meantime, we will just have to lock ourselves to version 4.0.4.

By the way, I did a quick preliminary test on a modified version of the def self.reload_app? and used part of what Rails 3.2.21 had and it seems to work like a charm :-) . Obviously, this is probably not ideal but at least it shows that there is possibly a way to add an additional check.

def self.reload_app?
  defined?(ActionDispatch::Reloader) && 
       Rails.application.config.cache_classes == false && 
       Rails.application.reloaders.map(&:updated?).any?
end

Anyway, thank you again. I hope some of my information to you is useful.

[sergey-alekseev]

Hey guys. @mic-kul submitted a PR on ASoftCo/leaky-gems#4. Can you confirm delayed_job has a memory leak? If it has I'd appreciate if you also confirm versions and environments affected.

[albus522]

@sergey-alekseev DJ does not have a memory leak. We simply use Rails code reloading and when users have non-reload safe code things go poorly. The same thing would happen running a simple rails server in dev mode.

[rwadstein]

@sergey-alekseev . It is true that it is not a memory leak. In development mode (or rather when Rails.application.config.caches_classes = false, the rake task reloads in the frequent scheduled intervals and memory gets used up by each reload which happens even outside of DJ.

The only difference between what Rails and DJ does, is that Rails will try to only reload if it detects that something has changed as I mentioned earlier in this discussion.

Temporarily, we just turned on cache_classes in development mode when we went to Rails 4 since you have to use the newer version.

FYI: in Rails 4, there are checks for duplicating routes, so we don't get the increased memory as it just raises an exception in Rails when that happens and kills the DJ rake task.

[albus522]

We are using the same level of conditional that Rails is
https://github.com/rails/rails/blob/4-2-stable/actionpack/lib/action_dispatch/middleware/reloader.rb#L69

[rwadstein]

Actually, the rake task does not invoke that same code. Both rails middleware and DJ will call prepare!

[rwadstein]

But Rails middleware will run a block to update the @validated variable, where in DJ it does not.

https://github.com/collectiveidea/delayed_job/blob/v4.0.6/lib/delayed/worker.rb#L296-L298
https://github.com/rails/rails/blob/857263445035404786e1f9785b76f3f4ddb7ca12/actionpack/lib/action_dispatch/middleware/reloader.rb#L68-L69

[rwadstein]

Since DJ is just calling the class method ActionDispatch::Reloader.prepare!, the @validated variable always defaults to true here.

[Loremaster]

Is it possible to disable this reloading via some kind of flag? We have problems in our app with Mail Interceptor because of that: collectiveidea/delayed_job_active_record#120

[dgobaud]

I'm still seeing this problem and I have cache_classes set to true

ruby '2.2.2'
gem 'rails', '4.2.5'
delayed_job (4.0.6)
delayed_job_active_record (4.0.3)

It's just a steady march up...

[xhero]

An option to disable this would be appreciated. We are running a pretty complex app with many external deps and DJ will completely eat up all the mem in dev mode in less than 15 minutes. Unfortunately we cannot control how all the external pieces do their initializers for their engines. Also DJ uses an incredible amount of CPU by reloading the app every 5 sec.

[ypresto]

An option to disable this would be appreciated.

This is really appreciated because ActiveJob already reloads code as needed on every execution.

ActiveJob::Execution

      def execute(job_data) #:nodoc:
        ActiveJob::Callbacks.run_callbacks(:execute) do
          job = deserialize(job_data)
          job.perform_now
        end
      end

ActiveJob::Railtie

    initializer "active_job.set_reloader_hook" do |app|
      ActiveSupport.on_load(:active_job) do
        ActiveJob::Callbacks.singleton_class.set_callback(:execute, :around, prepend: true) do |_, inner|
          app.reloader.wrap do
            inner.call
          end
        end
      end
    end

ActiveSupport::Reloader

    # Run the supplied block as a work unit, reloading code as needed
    def self.wrap
      executor.wrap do
        super
      end
    end

[ypresto]

My workaround is to place below snippet to initializer.

# ActiveJob will reload codes if necessary. DelayedJob consumes CPU and memory for reloading on every 5 secs.
# TODO: https://github.com/collectiveidea/delayed_job/issues/776#issuecomment-307161178
Delayed::Worker.instance_exec do
  def self.reload_app?
    false
  end
end

[thisduck]

Part of the problem here is that delayed_job reloads the code more often than it is needed. By default this is every 5 seconds (if you have cache_classes set to false). If you leave delayed_job running on your dev machine over night, for instance, it will have reloaded the code thousands of times.
https://github.com/collectiveidea/delayed_job/blob/master/lib/delayed/worker.rb#L187

The code only needs to be reloaded if there are any jobs to run. Unfortunately, there is no shared API method in Delayed::Backend::Base to check to see if there are any runnable jobs.

I've hacked delayed_job to do this check for my company's usage since we only use delayed_job_active_record and it has a ready_to_run relation we can check against.

If anyone is interested you can see the diff here: master...financeit:v.4.0.6_leak_fix

[ghiculescu]

@thisduck you might like our solution: #1115 (comment)