From c96ad6b91ed8be4150beabe9a3de404d352e9fca Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Wed, 5 Feb 2025 15:21:03 +0100 Subject: [PATCH 1/4] Make sure to close DB connections before fork --- app/models/solid_queue/record.rb | 5 +++++ lib/solid_queue/supervisor.rb | 2 ++ 2 files changed, 7 insertions(+) diff --git a/app/models/solid_queue/record.rb b/app/models/solid_queue/record.rb index d73e41b2..d88b833c 100644 --- a/app/models/solid_queue/record.rb +++ b/app/models/solid_queue/record.rb @@ -6,6 +6,11 @@ class Record < ActiveRecord::Base connects_to(**SolidQueue.connects_to) if SolidQueue.connects_to + def self.clear_all_connections! + self.connection_handler.clear_all_connections!(:writing) + self.connection_handler.clear_all_connections!(:reading) + end + def self.non_blocking_lock if SolidQueue.use_skip_locked lock(Arel.sql("FOR UPDATE SKIP LOCKED")) diff --git a/lib/solid_queue/supervisor.rb b/lib/solid_queue/supervisor.rb index f2207691..53a46017 100644 --- a/lib/solid_queue/supervisor.rb +++ b/lib/solid_queue/supervisor.rb @@ -80,6 +80,8 @@ def start_process(configured_process) instance.mode = :fork end + Record.clear_all_connections! + pid = fork do process_instance.start end From 1ddb833b8c6dddf742b2ff5b2798b5ac933f6049 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Thu, 6 Feb 2025 00:39:36 +0100 Subject: [PATCH 2/4] Try disconnecting before each fork made in the lib --- lib/puma/plugin/solid_queue.rb | 2 +- lib/solid_queue.rb | 5 +++++ lib/solid_queue/supervisor.rb | 4 +--- test/integration/puma/plugin_test.rb | 2 +- test/test_helpers/processes_test_helper.rb | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/puma/plugin/solid_queue.rb b/lib/puma/plugin/solid_queue.rb index eca5fa5f..dd52e127 100644 --- a/lib/puma/plugin/solid_queue.rb +++ b/lib/puma/plugin/solid_queue.rb @@ -12,7 +12,7 @@ def start(launcher) end launcher.events.on_booted do - @solid_queue_pid = fork do + @solid_queue_pid = SolidQueue.safe_fork do Thread.new { monitor_puma } SolidQueue::Supervisor.start end diff --git a/lib/solid_queue.rb b/lib/solid_queue.rb index 02b88d05..25de0869 100644 --- a/lib/solid_queue.rb +++ b/lib/solid_queue.rb @@ -73,5 +73,10 @@ def instrument(channel, **options, &block) ActiveSupport::Notifications.instrument("#{channel}.solid_queue", **options, &block) end + def safe_fork(&block) + Record.clear_all_connections! + fork { block.call } + end + ActiveSupport.run_load_hooks(:solid_queue, self) end diff --git a/lib/solid_queue/supervisor.rb b/lib/solid_queue/supervisor.rb index 53a46017..219f2cae 100644 --- a/lib/solid_queue/supervisor.rb +++ b/lib/solid_queue/supervisor.rb @@ -80,9 +80,7 @@ def start_process(configured_process) instance.mode = :fork end - Record.clear_all_connections! - - pid = fork do + pid = SolidQueue.safe_fork do process_instance.start end diff --git a/test/integration/puma/plugin_test.rb b/test/integration/puma/plugin_test.rb index bac98a2b..fa791b59 100644 --- a/test/integration/puma/plugin_test.rb +++ b/test/integration/puma/plugin_test.rb @@ -15,7 +15,7 @@ class PluginTest < ActiveSupport::TestCase -s config.ru ] - @pid = fork do + @pid = SolidQueue.safe_fork do exec(*cmd) end end diff --git a/test/test_helpers/processes_test_helper.rb b/test/test_helpers/processes_test_helper.rb index 9a6d0f65..3704e7dd 100644 --- a/test/test_helpers/processes_test_helper.rb +++ b/test/test_helpers/processes_test_helper.rb @@ -2,7 +2,7 @@ module ProcessesTestHelper private def run_supervisor_as_fork(**options) - fork do + SolidQueue.safe_fork do SolidQueue::Supervisor.start(**options.with_defaults(skip_recurring: true)) end end From 4e3270653f45ee509d58e127550aa14c5ad50116 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Thu, 6 Feb 2025 17:06:37 +0100 Subject: [PATCH 3/4] Use _fork hook instead of custom fork method --- lib/puma/plugin/solid_queue.rb | 2 +- lib/solid_queue.rb | 7 ++----- lib/solid_queue/fork_safety.rb | 15 +++++++++++++++ lib/solid_queue/supervisor.rb | 2 +- test/integration/puma/plugin_test.rb | 2 +- test/test_helpers/processes_test_helper.rb | 2 +- 6 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 lib/solid_queue/fork_safety.rb diff --git a/lib/puma/plugin/solid_queue.rb b/lib/puma/plugin/solid_queue.rb index dd52e127..eca5fa5f 100644 --- a/lib/puma/plugin/solid_queue.rb +++ b/lib/puma/plugin/solid_queue.rb @@ -12,7 +12,7 @@ def start(launcher) end launcher.events.on_booted do - @solid_queue_pid = SolidQueue.safe_fork do + @solid_queue_pid = fork do Thread.new { monitor_puma } SolidQueue::Supervisor.start end diff --git a/lib/solid_queue.rb b/lib/solid_queue.rb index 25de0869..fb90e946 100644 --- a/lib/solid_queue.rb +++ b/lib/solid_queue.rb @@ -20,6 +20,8 @@ module SolidQueue extend self + ForkSafety # add fork safety hooks + DEFAULT_LOGGER = ActiveSupport::Logger.new($stdout) mattr_accessor :logger, default: DEFAULT_LOGGER @@ -73,10 +75,5 @@ def instrument(channel, **options, &block) ActiveSupport::Notifications.instrument("#{channel}.solid_queue", **options, &block) end - def safe_fork(&block) - Record.clear_all_connections! - fork { block.call } - end - ActiveSupport.run_load_hooks(:solid_queue, self) end diff --git a/lib/solid_queue/fork_safety.rb b/lib/solid_queue/fork_safety.rb new file mode 100644 index 00000000..2c6abd53 --- /dev/null +++ b/lib/solid_queue/fork_safety.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module SolidQueue + module ForkSafety + def _fork + Record.clear_all_connections! + + pid = super + + pid + end + end +end + +Process.singleton_class.prepend(SolidQueue::ForkSafety) diff --git a/lib/solid_queue/supervisor.rb b/lib/solid_queue/supervisor.rb index 219f2cae..f2207691 100644 --- a/lib/solid_queue/supervisor.rb +++ b/lib/solid_queue/supervisor.rb @@ -80,7 +80,7 @@ def start_process(configured_process) instance.mode = :fork end - pid = SolidQueue.safe_fork do + pid = fork do process_instance.start end diff --git a/test/integration/puma/plugin_test.rb b/test/integration/puma/plugin_test.rb index fa791b59..bac98a2b 100644 --- a/test/integration/puma/plugin_test.rb +++ b/test/integration/puma/plugin_test.rb @@ -15,7 +15,7 @@ class PluginTest < ActiveSupport::TestCase -s config.ru ] - @pid = SolidQueue.safe_fork do + @pid = fork do exec(*cmd) end end diff --git a/test/test_helpers/processes_test_helper.rb b/test/test_helpers/processes_test_helper.rb index 3704e7dd..9a6d0f65 100644 --- a/test/test_helpers/processes_test_helper.rb +++ b/test/test_helpers/processes_test_helper.rb @@ -2,7 +2,7 @@ module ProcessesTestHelper private def run_supervisor_as_fork(**options) - SolidQueue.safe_fork do + fork do SolidQueue::Supervisor.start(**options.with_defaults(skip_recurring: true)) end end From 57036dcebcf930c20cda67f6ce1bf15aec793ca8 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Fri, 7 Feb 2025 23:23:57 +0100 Subject: [PATCH 4/4] just sleep less? --- test/integration/processes_lifecycle_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/processes_lifecycle_test.rb b/test/integration/processes_lifecycle_test.rb index ebb1100c..69086761 100644 --- a/test/integration/processes_lifecycle_test.rb +++ b/test/integration/processes_lifecycle_test.rb @@ -127,7 +127,7 @@ class ProcessesLifecycleTest < ActiveSupport::TestCase signal_process(@pid, :TERM, wait: 0.5) - sleep(SolidQueue.shutdown_timeout + 0.5.second) + sleep(SolidQueue.shutdown_timeout + 0.1.second) assert_completed_job_results("no pause") assert_job_status(no_pause, :finished)