Enable hot-reload via adding ActiveSupport::Reloader delegate

[littleali]

This PR is related to #720 and it is same as #627 and #725

Pull Request Detail (Copied from #627)

This change adds explicit integration for Rails autoloading. Rails autoloading already partially worked in Shoryuken in that constants that weren't eager loaded were automatically autoloaded. Shoryuken, however, didn't respect config.cache_classes. This meant that once a constant was loaded by the Shoryuken process, it would never be unloaded and reloaded. If you changed the code in a worker class after Shoryuken had loaded it, the change would have no effect until you restarted the Shoryuken process.

The proposed change uses Rails.application.reloader, which takes care of safely reloading constants. This is the same technique employed by active_job and sidekiq and even explicitly documented in the Rails guides. The reloader takes the appropriate locks to ensure that no two threads ever simultaneously unload or reload constants. If the application developer modifies code in an autoloaded directory (including the implementation of workers themselves in app/workers), the changes will take effect the next time a job is processed.

Breaking changes

This change probably warrants a major version bump. The reason for that is that if Shoryuken library code has any reference to objects that belong to autoloaded modules, it's quite easy to run into autoloading errors. If other people's apps are like my own, this is most likely to happen with custom middleware. This is an example from a Rails initializer:

Shoryuken.configure_server do |config|
  config.server_middleware do |chain|
    chain.add MyCustomMiddleware
  end
end

Assume MyCustomMiddleware is defined in an autoloaded directory (basically anything under app, in addition to directories explicitly added to config.autoload_paths), and that the execution of MyCustomMiddleware#call has unqualified references to autoloaded constants. In this case, if Shoryuken tries to invoke that middleware after the application developer has triggered reloading (by modifying files in autoloaded directories), then autoloading will fail with A copy of MyCustomMiddleware has been removed from the module tree but is still active!.

The fix is to ensure that Shoryuken library code never retains a reference to any object defined in autoloaded modules and whose code has references to autoloaded modules. In the example of custom middleware, the easiest solution is to just attach the middleware to workers instead of the global server_middleware. You can even do this in your base worker class.

class ApplicationWorker
  include Shoryuken::Worker

  def self.inherited(klass)
    klass.server_middleware do |chain|
      chain.add MyCustomMiddleware
    end
  end
end

This works because the worker-specific middleware chain has a lifetime associated with the worker class. Once unloading happens, the old references to MyCustomMiddleware are unreachable by the Shoryuken process. When a new job comes in, Shoryuken autoloads the worker class, and the inherited block will run, constructing a new instance of the MyCustomMiddleware class through autoloading.

Users upgrading to a version of Shoryuken that includes this change can, in theory, alternatively just turn on config.cache_classes in their Shoryuken server process (even in development). The experience will be the same as before in that classes won't be reloaded, but it might be an easier upgrade path for some.

Shoryuken.configure_server do
  Rails.application.configure do
    config.cache_classes = true
  end
end

[phstc]

hi @littleali

Thank you for the PR explanation! This is very helpful.

I was wondering if instead of a major bump, we should enable cache_classes = true or any other way of making reloading optional.

Shoryuken.configure_server do
  Rails.application.configure do
    config.cache_classes = true
  end
end

I believe most people don't define middleware like this

  def self.inherited(klass)
    klass.server_middleware do |chain|
      chain.add MyCustomMiddleware
    end
  end

I'm worried that the change may take some solid debugging time to figure out the issue with the middleware definition.

Any thoughts?

[littleali]

Hi @phstc

Regarding your comment I've added a flag called enable_reloading to control reloading. The default value is false which means this non-breaking change provides more control over reloading behaviour. In addition to the code changes, I've also updated the project's wiki to include documentation on this new flag. This should provide guidance to developers on how and when to use this feature. Finally, I've updated the existing RSpec tests to ensure this new functionality is covered and works as expected.

[phstc]

@littleali thank you for the explanation and pull request. The new version has been released with your changes. Can you update the wiki?