Rails' eager load hooks at not invoked

[jonleighton]

Sidekiq triggers eager loading by calling Rails.application.eager_load!. This causes all the files in config.eager_load_paths to be loaded, but it doesn't allow the before_eager_load hook to run, nor does it result in the eager_load! methods of the config.eager_load_namespaces being called.

To do that, we need config.eager_load to be set.

Rather than loading config/environment.rb only, Sidekiq could load config/application.rb, set Rails.application.config.eager_load = true, and then load config/environment.rb. I believe this would have the desired effect. We do something similar in spring.

In the meantime, here's a workaround I'm using:

Sidekiq.configure_server do |config|
  Rails.application.config.eager_load = true
end

Cheers

[mperham]

How's that look?

[mperham]

@jonleighton Can you double check and close this issue if it looks correct?

[jonleighton]

Cheers, will test it against our app

[jonleighton]

Ok, setting Rails.application.config.eager_load = true doesn't work, because that gets overwritten in e.g. config/environments/development.rb which happens later. This should work (I tried it):

Rails::Application.initializer "sidekiq.eager_load" do
  Rails.application.config.eager_load = true
end

The block gets run after config/environments/development.rb runs, but before rails decides whether to perform eager loading.

[mperham]

Ok, try master again.

[jonleighton]

Yep, that works

[fabiokr]

I just updated an app to Sidekiq 3.2.0, and starting seeing NameError: uninitialized constant ... under the dev environment. Any guesses? Any other info I can provide to help debugging the issue?

[fabiokr]

A couple additional info: the uninitialized constant mentioned above is an ActiveRecord model under app/models inside a Rails engine.

[mperham]

Cc @jonleighton

On Jul 4, 2014, at 6:28, Fabio Kreusch notifications@github.com wrote:

A couple additional info: the uninitialized constant mentioned above is an ActiveRecord model under app/models inside a Rails engine.

—
Reply to this email directly or view it on GitHub.

[jonleighton]

@fabiokr are you seeing the issue inside the Sidekiq process? do you have a standard config/{application,environment}.rb? does it work the first time then fail on subsequent jobs?

[fabiokr]

@jonleighton Btw, this is Rails 3.2.19.

Yes, it happens under the Sidekiq process.

For the specific environment files, we share some of the configurations within the engine like this:

# the_app/config/environments/development.rb
require 'the_engine/engine/config/development'

TheApp::Application.configure do
end

# the_engine/engine/config/development.rb
Rails.application.class.configure do
  config.cache_classes = false
  config.whiny_nils = true
  config.consider_all_requests_local       = true
  config.action_controller.perform_caching = false
  config.action_mailer.raise_delivery_errors = false
  config.active_support.deprecation = :log
  config.action_dispatch.best_standards_support = :builtin
  config.active_record.auto_explain_threshold_in_seconds = 0.5
  config.assets.compress = false
  config.assets.debug = true
end

Also, it fails in the first time a job is run and on subsequent runs as well.

[jonleighton]

Right, I've realised that the solution I suggested is only compatible with Rails 4+, since config.eager_load was only added in Rails 4. Sorry about that. @mperham what do you want to do? I guess the options are:

1. Drop support for Rails 3.2
2. Add extra hacks to support both Rails 3.2 and Rails 4+

[ryansch]

👍 for extra hacks. We're still on Rails 3.2 at the moment. Somehow we still have dependencies that aren't ready for Rails 4 and we haven't had time to hack on them or find replacements.

/cc @morganick

[mperham]

I can't really drop 3.2 without a major version bump. When does 3.2 go EOL?

On Jul 4, 2014, at 14:13, Jon Leighton notifications@github.com wrote:

Right, I've realised that the solution I suggested is only compatible with Rails 4+, since config.eager_load was only added in Rails 4. Sorry about that. @mperham what do you want to do? I guess the options are:

Drop support for Rails 3.2
Add extra hacks to support both Rails 3.2 and Rails 4+
—
Reply to this email directly or view it on GitHub.

[mperham]

Looks like Rails 3.2 is in "severe security fixes only" mode but Sidekiq 3 shouldn't change supported versions. We need a Sidekiq 3.2.1 release which works on Rails 3.2, so reverting is probably the right thing to do here.

I'm happy to have a 4-x branch with these changes which is Rails 4.0+ only. We'll make that branch master after a month or so to let 3.2.x settle. Anyone want to do that?

[j-manu]

And keep around a branch for rails 3.2 which will be bug/security fix only?

[mperham]

Yes, once master becomes 4.x only, we'd create a 3-x branch and find a volunteer to maintain the branch.

[jonleighton]

That plan sounds fine to me. Sorry about the breakage.

[stouset]

This patch seems to have caused a double-loading issue, at least in Ruby 2.1.1. Requiring config/environment.rb causes config/application.rb to be loaded a second time, possibly because the path given to require in config/environment.rb and sidekiq/cli.rb are different.

[mperham]

@stouset Please prove it with a simple app which reproduces the problem.

[mperham]

I say prove it because 5 minutes of me trying couldn't reproduce any constant redefinition warnings.

[stouset]

I investigated further; not sure if this should be considered a bug in sidekiq or Rails, or if it's working as intended.

The problem is when a file in an eager loaded directory (e.g., app/*) is explicitly require'd. In this case, I'm requiring a file that doesn't obey Rails' autoloading conventions (a generated file compiled from a protobuf definition). During sidekiq initialization, the file is getting explicitly require'd while eager loading, but then it's getting loaded afterward which causes the file to be evaluated again.

It's probably not a bug in sidekiq, but it does seem to be exposed by the eager loading hoops being jumped through here. I can provide a POC, but wanted to get your thoughts on if it's something you think sidekiq is responsible for first.

[mperham]

General rule of thumb: if your application code is using load at all, that's a problem.

[stouset]

It's not the application code. The application code is using require. Rails uses load when performing eager loading, which causes this problem if the files were required before being eager loaded. This appears to happen as a result of Sidekiq's approach to triggering eager loading.

[mperham]

Why is your application requiring it if Rails will eager load it for you anyways?

[jonleighton]

Try switching the require to require_dependency

On 17/09/14 05:31, Mike Perham wrote:

Why is your application requiring it if Rails will eager load it for you
anyways?

—
Reply to this email directly or view it on GitHub
#1791 (comment).

[stouset]

Why is your application requiring it if Rails will eager load it for you anyways?

As I said earlier,

The problem is when a file in an eager loaded directory (e.g., app/*) is explicitly require'd. In this case, I'm requiring a file that doesn't obey Rails' autoloading conventions (a generated file compiled from a protobuf definition).

Rails' eager loading will load the file, as it preemptively calls load on every file in every subdirectory in eager loaded paths. Rails' autoloading does not load the file, because it does not obey Rails' autoloading naming conventions (which is outside of my control). In the development environment, eager loading is disabled and Rails uses autoloading. In production and sidekiq, eager loading is used.

Try switching the require to require_dependency

All require_dependency does is ensures that constants defined by the required file are reloaded when config.cache_classes == false. This does not solve the problem that eager loading causes, in this case, the file to be loaded twice.

Again, I'm unsure whether or not this is a bug in Rails, a bug in sidekiq, or "works as intended". At the very least, the behavior only seems to be triggered by sidekiq due to the hoops it jumps through to ensure Rails' classes are eager loaded.

[mperham]

Thanks for your patience and explanation. This is a tricky situation.

What about moving the file out of the eager load path and doing a require as normal in your initializer?

[stouset]

Yes, I did that as a workaround (moved to lib/protos from app/protos), which solved the problem.

[jonleighton]

Right, interesting. I think I'd consider it a bug a Rails to be honest,
as I don't see a good reason for Rails' eager loading to use load. If
it used require this would presumably not be a problem.

On 17/09/14 19:26, Stephen Touset wrote:

Why is your application requiring it if Rails will eager load it for
you anyways?

As I said earlier,

The problem is when a file in an eager loaded directory (e.g.,
app/*) is explicitly require'd. In this case, I'm requiring a file
that doesn't obey Rails' autoloading conventions (a generated file
compiled from a protobuf definition).

Rails' /eager loading/ will load the file, as it preemptively calls
|load| on every file in every subdirectory in eager loaded paths. Rails'
/autoloading/ does not load the files, because they do not obey Rails'
autoloading naming conventions (which is outside of my control). In the
development environment, eager loading is disabled and Rails uses
autoloading. In production and sidekiq, eager loading is used.

Try switching the require to require_dependency

All |require_dependency| does is ensures that constants defined by the
required file are reloaded when |config.cache_classes == false|. This
does not solve the problem that eager loading causes, in this case, the
file to be loaded twice.

Again, I'm unsure whether or not this is a bug in Rails, a bug in
sidekiq, or "works as intended". At the very least, the behavior only
seems to be triggered by sidekiq due to the hoops it jumps through to
ensure Rails' classes are eager loaded.

—
Reply to this email directly or view it on GitHub
#1791 (comment).

[irongaze]

+1 for resolving this issue for Rails 3. I'm seeing the same issue - installed Sidekiq, ran bundle exec sidekiq, got double-loading errors from classes that need to be force-loaded in dev mode to allow for some meta-programming. As this method works for dev, test & production use in Rails, it seems that the issue should be handled on the Sidekiq side of things.

What is the purpose behind eager-loading after the environment require?

[ryanclark2]

Using Rails 4.2.0 we still encountered this issue when explicitly requiring a file that also fell into the eager_load path. Our solution was to add

ActiveSupport::Dependencies.mechanism = :require

to our sidekiq.rb initializer, which will tell Rails autoload and eager_load to use require rather than load.

[jfelchner]

We are also getting a double-loading issue. Tons of warnings about constants being reinitialized.

We have a requirement to use require in certain places in our code which is evidently causing this problem. The app boots fine (no warnings) using only Rails but as soon as we bundle exec sidekiq it blows up.

We now have a requirement to require a couple of our models explicitly so that certain code is loaded before other code and now the app won't boot at all due to errors being raised instead of warnings.

[brianhempel]

We experienced a different issue caused by Sidekiq's hacky Rails loading.

We set up a config/initializers/sidekiq.rb file with:

Rails.application.configure do
  config.active_job.queue_adapter = :sidekiq
end

This initializer worked great for our web server and other processes. They enqueue jobs to Sidekiq fine. But we discovered that within the Sidekiq worker process only the initializer was running but not taking effect. Within a bundle exec sidekiq process, the ActiveJob::Base.queue_adapter ended up as still the inline adapter, even though I can confirm that the above three lines of code were all being run during Sidekiq's startup.

Consequently, when a job tried to enqueue other jobs, they actually ran inline.

What fixed it for us was moving the code above to the config/application.rb file.

My understanding is that calling Rails.application.configure in an initializer is considered an acceptable practice, so it's unfortunate that in some cases Sidekiq seems to frustrate that pattern.

I believe calling Rails.application.configure in an initializer is a common practice. It's not clear what inside the Sidekiq boot process is causing the issue.

[ryansch]

I was also having issues with the way Sidekiq boots rails 4. I ended up making the following change to my development.rb.

config.cache_classes = !!Sidekiq.server?
config.eager_load = !!Sidekiq.server?

This had the benefit of running sidekiq in allow_concurrency mode.

It seems the existing sidekiq initializer runs too late for some of the boot code in our rails app.

[shime]

Had a custom directory inside app/models (eg. app/models/foo-bar/) that was throwing errors when starting Sidekiq in development.

wrong constant name Foo-bar
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/core_ext/module/qualified_const.rb:30:in `const_defined?'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/core_ext/module/qualified_const.rb:30:in `block in qualified_const_defined?'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/core_ext/module/qualified_const.rb:29:in `each'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/core_ext/module/qualified_const.rb:29:in `inject'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/core_ext/module/qualified_const.rb:29:in `qualified_const_defined?'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:376:in `qualified_const_defined?'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:145:in `block in watch_namespaces'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:143:in `map'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:143:in `watch_namespaces'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:643:in `new_constants_in'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:456:in `load_file'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:354:in `require_or_load'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:317:in `depend_on'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:233:in `require_dependency'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/engine.rb:472:in `block (2 levels) in eager_load!'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/engine.rb:471:in `each'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/engine.rb:471:in `block in eager_load!'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/engine.rb:469:in `each'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/engine.rb:469:in `eager_load!'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/engine.rb:346:in `eager_load!'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/application/finisher.rb:56:in `each'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/application/finisher.rb:56:in `block in <module:Finisher>'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/initializable.rb:30:in `instance_exec'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/initializable.rb:30:in `run'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/initializable.rb:55:in `block in run_initializers'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:226:in `block in tsort_each'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:348:in `block (2 levels) in each_strongly_connected_component'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:429:in `each_strongly_connected_component_from'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:347:in `block in each_strongly_connected_component'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:345:in `each'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:345:in `call'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:345:in `each_strongly_connected_component'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:224:in `tsort_each'
/Users/hrvoje/.rubies/ruby-2.2.0/lib/ruby/2.2.0/tsort.rb:203:in `tsort_each'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/initializable.rb:54:in `run_initializers'
/Users/hrvoje/.gem/ruby/2.2.0/gems/railties-4.2.0/lib/rails/application.rb:352:in `initialize!'
/Users/hrvoje/code/work/lessonhub/backend/config/environment.rb:5:in `<top (required)>'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:274:in `require'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:274:in `block in require'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:240:in `load_dependency'
/Users/hrvoje/.gem/ruby/2.2.0/gems/activesupport-4.2.0/lib/active_support/dependencies.rb:274:in `require'
/Users/hrvoje/.gem/ruby/2.2.0/gems/sidekiq-3.4.2/lib/sidekiq/cli.rb:241:in `boot_system'
/Users/hrvoje/.gem/ruby/2.2.0/gems/sidekiq-3.4.2/lib/sidekiq/cli.rb:50:in `run'
/Users/hrvoje/.gem/ruby/2.2.0/gems/sidekiq-3.4.2/bin/sidekiq:13:in `<top (required)>'
/Users/hrvoje/.gem/ruby/2.2.0/bin/sidekiq:23:in `load'
/Users/hrvoje/.gem/ruby/2.2.0/bin/sidekiq:23:in `<main>'

Doing exactly what @ryansch resolved it.

[pmatsinopoulos]

I am late here. But can I ask why does Sidekiq asks Rails to work in eager load mode and does not let Rails environment decide about that? I am referring to this line here:

https://github.com/mperham/sidekiq/blob/master/lib/sidekiq/cli.rb#L236

[mperham]

eager loading is required in Rails 4 because Sidekiq is multithreaded.

[pmatsinopoulos]

1. I guess that you are referring to the fact that Sidekiq might have many threads trying to load the same constant at the same time. In that case, having all the eager loaded paths constants loaded in advance would solve that problem.

IMHO, Sidekiq should allow Rails environment decide whether to eager load or not. For example, on development environment, not to eager load. Also, on development environment using 1 thread is pretty fine. But even if developer wants to use many threads, failing to load a constant by a thread due to contention by another, wouldn't happen too often, and wouldn't be of a big problem. Developer could live with that. On the other hand, on production environment, Rails eager loads anyway.

Please, note also that multithreaded problem in the context of loading of constants is not solved with the eager loading at Sidekiq (nor at Rails) level anyway. There still might be constants that are needed by Sidekiq, but Rails developer might not have put them inside the eager loaded paths.

In summary, I do not find a reason to make cli Sidekiq code ugly. Doesn't worth it.

2. And there are warnings on development environment. With Sidekiq asking Rails to eager load, same file might be loaded multiple times, because cache_classes is false. In case you want to keep the line that asks Rails to eager load, have you considered finding a way to set the cache_classes to true so that files are loaded only once (with cache_classes to true, they are required not loaded)?

[mperham]

@pmatsinopoulos I have no interest in changing the behavior for Rails 3 and 4. You might be interested in this pull request #2457