Non-forking implementation

[headius]

Hello! I would like to offer my help making modifications to Solid Queue to support executing in-process using threads rather than out-of-process as a fork.

There's many issues with using fork:

• Impossible to support on JRuby and TruffleRuby, as well as on Windows
• Wastes system resources on CRuby by creating additional processes with their own working sets, GCs, and JITs.

The primary reason for using forks here is to get parallelism, which can be achieved easily by using threads on JRuby and TruffleRuby. The logic you have for starting up, managing, and terminating forked processes can be modified to do the same with threads, and no additional processes would be needed.

I have started playing with patches locally that swap calls to fork for calls to Thread.new with appropriate thread management code, and there's not really even that many places that would need to be changed. The bulk of the logic is all signals and waitpid, which map pretty easily to Thread#kill, Thread#raise, and Thread#join (or just let it die quietly).

Could we collaborate on this? I could start hacking on a branch this week.

[headius]

I have not actually gotten Solid Queue to start up under JRuby, since I'm not a Rails guy and I don't know how to set that up. But the patch below is a ham-fisted attempt to swap forks for threads, and may be illustrative.

diff --git a/lib/puma/plugin/solid_queue.rb b/lib/puma/plugin/solid_queue.rb
index 434b8f6..6c8d547 100644
--- a/lib/puma/plugin/solid_queue.rb
+++ b/lib/puma/plugin/solid_queue.rb
@@ -13,20 +13,20 @@ Puma::Plugin.create do
 
     if Gem::Version.new(Puma::Const::VERSION) < Gem::Version.new("7")
       launcher.events.on_booted do
-        @solid_queue_pid = fork do
-          Thread.new { monitor_puma }
-          SolidQueue::Supervisor.start
-        end
+        @solid_queue_pid = #fork do
+          # Thread.new { monitor_puma }
+          Thread.new { SolidQueue::Supervisor.start }
+        # end
       end
 
       launcher.events.on_stopped { stop_solid_queue }
       launcher.events.on_restart { stop_solid_queue }
     else
       launcher.events.after_booted do
-        @solid_queue_pid = fork do
-          Thread.new { monitor_puma }
-          SolidQueue::Supervisor.start
-        end
+        @solid_queue_pid = #fork do
+          # Thread.new { monitor_puma }
+          Thread.new { SolidQueue::Supervisor.start }
+        # end
       end
 
       launcher.events.after_stopped { stop_solid_queue }
@@ -36,10 +36,12 @@ Puma::Plugin.create do
 
   private
     def stop_solid_queue
-      Process.waitpid(solid_queue_pid, Process::WNOHANG)
+      # Process.waitpid(solid_queue_pid, Process::WNOHANG)
       log "Stopping Solid Queue..."
-      Process.kill(:INT, solid_queue_pid) if solid_queue_pid
-      Process.wait(solid_queue_pid)
+      solid_queue_pid.kill
+      solid_queue_pid.join
+      # Process.kill(:INT, solid_queue_pid) if solid_queue_pid
+      # Process.wait(solid_queue_pid)
     rescue Errno::ECHILD, Errno::ESRCH
     end
 
diff --git a/lib/solid_queue/supervisor.rb b/lib/solid_queue/supervisor.rb
index ef9c79d..89fdfe5 100644
--- a/lib/solid_queue/supervisor.rb
+++ b/lib/solid_queue/supervisor.rb
@@ -80,7 +80,7 @@ module SolidQueue
           instance.mode = :fork
         end
 
-        pid = fork do
+        pid = Thread.new do
           process_instance.start
         end
 
@@ -130,20 +130,22 @@ module SolidQueue
       end
 
       def term_forks
-        signal_processes(forks.keys, :TERM)
+        forks.each(&:kill)
+        # signal_processes(forks.keys, :TERM)
       end
 
       def quit_forks
-        signal_processes(forks.keys, :QUIT)
+        forks.each(&:kill)
+        # signal_processes(forks.keys, :QUIT)
       end
 
       def reap_and_replace_terminated_forks
-        loop do
-          pid, status = ::Process.waitpid2(-1, ::Process::WNOHANG)
-          break unless pid
-
-          replace_fork(pid, status)
-        end
+        # loop do
+        #   pid, status = ::Process.waitpid2(-1, ::Process::WNOHANG)
+        #   break unless pid
+        #
+        #   replace_fork(pid, status)
+        # end
       end
 
       def reap_terminated_forks

I suspect that the thread-based version will end up being quite a bit simpler than the fork-based version, since there's less hassle monitoring the supervisor and the lifecycle of the single process will apply to all worker threads (so no monitoring of a parent puma process).

[dhh]

Don't see how a thread based version is going to give us the throughput we need. Just like running puma on threads alone wouldn't. GVL is going to be a big factor here. But possible that a Ractor-based solution might work. Feel free to explore that and open a PR with benchmarks if you get somewhere with it.

[headius]

@dhh My interest is for users of parallel-threaded implementations of Ruby like JRuby. We have had users and potential users asking what it would take for Solid Queue to work on JRuby.

I think the answer is "not very much", and the threaded version would likely have less overhead than the forked version via shared memory and shared VM resources. A thread-based version would give you all the throughput you need in the same JRuby process without forking any additional processes.

[eregon]

From reading the issue, I think there is some misunderstanding here: Solid Queue would need to keep a fork-based implementation for CRuby, to get parallelism on CRuby (or use Ractor instead of fork, though that probably limits quite a bit what can run inside a job).

And this issue as I understand it is about adding a Thread-based implementation, which would be used where Process.respond_to?(:fork) is false (JRuby, TruffleRuby, Windows, etc). I think that would be valuable.

[dhh]

Happy to accommodate JRuby, if we can make minimal incisions into the code to do so. But it's not a project that we as maintainers of Solid Queue will be taking on, as it's not a platform that we use. So whatever solution is proposed has to have no impact on CRuby running and be minimally invasive. If anyone is interested in pursuing that, please do open a PR 👍

[seuros]

Changing from Unix fork to threads will make it compatible with Java and Windows, but that totally changes the architecture. It will be a breaking change.

JVM Rubies have real parallel threads with no GVL and correct isolation. CRuby doesn't have that privilege.

The GVL means all threads are on one CPU core for Ruby execution, and if one thread crashes badly enough, the whole process is gone terminating the other jobs.

I think the correct approach is to have a monkey patcher gem, that will give every engine solution. CRuby stays vanilla, Jruby use java executors, and Truffle can use threads.

Windows ? They can install Omarchy/Omakub or test in production.

Full implementation here , i had it working for jruby, just added truffleruby.

The gem can be transferred to jruby org.

[eregon]

I think the correct approach is to have a monkey patcher gem, that will give every engine solution.

That's nice to prototype but it's far from convenient for users, they have to discover that gem and by default it just doesn't work and errors.
Having it integrated in this gem would make things just work, and ensure better testing by running all tests.
It's also more reliable by not relying on monkey-patching.

@seuros Would you be up to open a PR with your Executor from https://github.com/seuros/solid_queue-java/blob/master/lib/solid_queue/java/executor.rb here (used unless Process.respond_to?(:fork))? I think you could simplify to just the (Ruby) Thread-based executor, since that works fine on JRuby too, and is more familiar for Rubyists in general.

Another option would be to use a concurrent-ruby thread pool, since concurrent-ruby is already a dependency of solid_queue. That's a more complete and well-tested thread pool implementation.

[rosa]

Hey! There's already some preliminary work on this in #644, which was from a previous attempt I made myself, but before this was made Rails's default, and so way before version 1.0.0. It needs some work (the PR reinstitutes that former feature). The idea is that it'd be completely orthogonal to the fork-based version (the default). We could also make it conditional on Process.respond_to?(:fork).

[seuros]

@eregon I could do that!

But here’s my reasoning: this gem probably belongs in the JRuby org, alongside the other JDBC gems.

Rails isn’t exactly plug-and-play on JRuby: every ActiveRecord adapter has its own JDBC Wrapper.

The Java-based implementation I had works quite well for my use case, while the thread-based version comes from @headius in this PR.

Adding multiple (or worse, triple) implementations inside this gem would just add complexity.

Maintainers would then have to test against several JRuby and Java versions.

If @rosa thinks it’s better to keep it here, that is fine too.

The other gem isnt even released yet, it is just an extraction for now. Headius can decide its fate.

[rosa]

I think a version that uses threads instead of processes could be included here as per #644. My initial motivation to do it was to have a lighter version to use in development and in memory-contrained environments, but it could also support JRuby, even if that wasn't the original motivation. I think that'd be a matter of finishing #644, that still needs some work. If anyone is willing to do that work, I'd be happy to review!

[headius]

I'm willing to assist with either firming up the prototype from @seuros or helping to finish #644 If I knew that it would be accepted into the project. I was a bit discouraged that my issue was summarily closed without any discussion of the actual benefits and seemingly without actually reading my description.

Reopening this issue would help give me confidence that my work would not be ignored or discarded.

[rosa]

I'd say, from those two, only #644 would have a solid chance of being included. It's an open PR, and I generally close all PRs that I'm not willing to consider. Would that give you some more confidence?

[headius]

@rosa It does seem that #644 has momentum, and I'm glad to see there are additional reasons for a non-forking version even when only considering CRuby.

@seuros With JRuby-specific code I think your version would be unlikely to get merged as-is. Perhaps we can take what you have created and use that work to finish #644 on top of something like concurrent-ruby (already a Rails dependency)?