ruby-shoryuken / shoryuken

A super efficient Amazon SQS thread based message processor for Ruby. This project is in MAINTENANCE MODE. Reach me out on Slack link on the description if you want to become a new maintainer.
Other
2.06k stars 280 forks source link

Adding Rails Reloading Support #725

Closed littleali closed 1 year ago

littleali commented 1 year ago

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

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
github-actions[bot] commented 1 year ago

This PR is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

github-actions[bot] commented 1 year ago

This PR was closed because it hasn't seen activity for a while.