Skip to content
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

Fix memory leak - for idling workers #1115

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

kevinhq
Copy link

@kevinhq kevinhq commented May 18, 2020

The fix is based on our company's need but we think it may be useful for others in community too.

@ghiculescu
Copy link

Here is how we are approaching it:

# frozen_string_literal: true

# Reloads autoloadable code as needed around each job execution
# See  https://github.com/mperham/sidekiq/pull/2457 /
# https://github.com/mperham/sidekiq/blob/461ef8392fe8503cda849769fc590a113bd04559/lib/sidekiq/rails.rb#L6 for inspiration

module Delayed
  module Plugins
    class Reloader < Plugin
      callbacks do |lifecycle|
        lifecycle.around(:invoke_job) do |_job, *_args, &block|
          ::Rails.application.reloader.wrap do
            block.call
          end
        end
      end
    end
  end
end

Delayed::Worker.plugins << Delayed::Plugins::Reloader

module DJUseOurOwnReloader
  module ClassMethods
    def reload_app?
      false
    end
  end

  def self.prepended(mod)
    mod.singleton_class.prepend(ClassMethods)
  end
end

Delayed::Worker.prepend(DJUseOurOwnReloader)

An equivalent change to DJ would:

  • Remove the current reload? call.
  • Add the plugin instead.

@girishgl
Copy link

Could you please provide more context or information regarding the specific situation or scenario in which you have observed the memory leak in the Delayed Job gem? This will help me using same code change in my organisation.

Moreover I have seen this exception often times. DO you have any idea about this?
SIGTERM /gems/gems/delayed_job-4.1.3/lib/delayed/worker.rb:160:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants