[Proposal] Replace ConnectionReset to Rails executor

[le0pard]

Based on rails docs: https://guides.rubyonrails.org/threading_and_code_execution.html#executor

The Rails Executor separates application code from framework code: any time the framework invokes code you've written in your application, it will be wrapped by the Executor.

Here how it can be done in gruf:

app = Rails.application
executor = app.config.reload_classes_only_on_change ? app.reloader : app.executor

Gruf.configure do |c|
  c.interceptors.use(Gruf::Interceptors::RailsExecutor, executor: executor)
end

Interseptor itself (not tested - code provided as example):

module Gruf
  module Interceptors
    # Executor runs Rails executor for each call
    # See https://guides.rubyonrails.org/v5.2.0/threading_and_code_execution.html#framework-behavior
    class RailsExecutor < ::Gruf::Interceptors::ServerInterceptor

      def call
        options.executor.wrap { yield }
      end

    end
  end
end

In this case no need care about close connections for Activerecord and also fixing issues to other databases (like release connections to redis), which not covered by gruf, but covered by rails.

Also

Before entering the wrapped block, the Reloader will check whether the running application needs to be reloaded

So it may help to reach this https://github.com/bigcommerce/gruf/issues/115 and https://github.com/bigcommerce/gruf/issues/86

[le0pard]

Idea came from anycable:

https://github.com/anycable/anycable-rails/blob/4cff609db98713251df14e5e43feb4d4724f8da8/lib/anycable/rails/railtie.rb#L37-L42

and

https://github.com/anycable/anycable-rails/blob/master/lib/anycable/rails/middlewares/executor.rb

[splittingred]

@le0pard Thanks! This is interesting. It seems this is only supported in Rails 5.0+, do we have an idea of Rails version compatibility here? It does seem more promising than the current interceptor approach.

Some questions I have from the docs:

This operation will block temporarily if another thread is currently either autoloading a constant or unloading/reloading the application.

Would this potentially lock incoming gRPC requests during constant autoloading? I wonder how that would impact tail latencies on boot (potentially affecting "healthy status" checks in container schedulers).

Definitely worth an investigation. Thanks for the proposal.

[le0pard]

Thanks @splittingred

It seems this is only supported in Rails 5.0+, do we have an idea of Rails version compatibility here?

Looks like yes. For old rails we can leave ConnectionReset interseptor.

Would this potentially lock incoming gRPC requests during constant autoloading?

Maybe, this need to be checked. That is why I pointed to anycable, which have internal grpc server and found how they resolving such issue.

[ximus]

I did similar work on my fork to enable code reloading. The work from Anycable is likely better inspiration than mine as I worked quickly without much domain experience.

However I had to do one extra thing not mentioned here to get my controllers to reload. Currently, Gruf's ServiceBinder setups up a reference to controller class instances when the server initializes. Given how code reload works, this reference is never refreshed to point to newer (reloaded) class instances, this needs to be done explicitly.

There is probably a few ways to refresh this reference. I went the dumb simple way of doing a constant lookup. see here

reloader.to_prepare do
  controller = controller.name.constantize
end