Drop support for forking workers, use spawn by default

Make systemd optional on systemd enabled systems

Api/web service worker needs ui-classic as it can try to read an existing UI session, which
can contain serialized classes from ui-classic.

Previously, we tried removing fork here: ManageIQ#16130
It was reverted here: ManageIQ#16154

Some of the followups needed to fix the original problems including passing down
ems_id to per ems workers, resolved in:
ManageIQ#16199 and
ManageIQ#18648

At this point, things should just work.

Gemfile

@@ -57,7 +57,6 @@ gem "manageiq-postgres_ha_admin",     "~>3.1",         :require => false
 gem "memoist",                        "~>0.15.0",      :require => false
 gem "mime-types",                     "~>3.0",         :path => File.expand_path("mime-types-redirector", __dir__)
 gem "more_core_extensions",           "~>3.7"
-gem "nakayoshi_fork",                 "~>0.0.3"  # provides a more CoW friendly fork (GC a few times before fork)
 gem "net-ldap",                       "~>0.16.1",      :require => false
 gem "net-ping",                       "~>1.7.4",       :require => false
 gem "openscap",                       "~>0.4.8",       :require => false

app/models/miq_web_service_worker.rb

@@ -17,7 +17,10 @@ def self.supports_container?
   end
 
   def self.bundler_groups
-    %w[manageiq_default graphql_api]
+    # TODO: The api process now looks at the existing UI session as of: https://github.com/ManageIQ/manageiq-api/pull/543
+    # ui-classic should not be but is serialializing its classes into session, so we need to have access to them for deserialization
+    # sandboxes;FC:-ActiveSupport::HashWithIndifferentAccess{I"dashboard;FC;q{I"perf_options;FS:0ApplicationController::Performance::Options$typ0:daily_date0:hourly_date0: days0:
+    %w[manageiq_default ui_dependencies graphql_api]
   end
 
   def self.kill_priority

app/models/miq_worker.rb

@@ -307,48 +307,6 @@ def send_message_to_worker_monitor(message, *args)
     )
   end
 
-  def self.before_fork
-    preload_for_worker_role if respond_to?(:preload_for_worker_role)
-  end
-
-  def self.after_fork
-    close_pg_sockets_inherited_from_parent
-    DRb.stop_service
-    close_drb_pool_connections
-    renice(Process.pid)
-    CodeCoverage.run_hook
-  end
-
-  # When we fork, the children inherits the parent's file descriptors
-  # so we need to close any inherited raw pg sockets in the child.
-  def self.close_pg_sockets_inherited_from_parent
-    owner_to_pool = ActiveRecord::Base.connection_handler.instance_variable_get(:@owner_to_pool)
-    owner_to_pool[Process.ppid].values.compact.each do |pool|
-      pool.connections.each do |conn|
-        socket = conn.raw_connection.socket
-        _log.info("Closing socket: #{socket}")
-        IO.for_fd(socket).close
-      end
-    end
-  end
-
-  # Close all open DRb connections so that connections in the parent's memory space
-  # which is shared due to forking the child process do not pollute the child's DRb
-  # connection pool.  This can lead to errors when the children connect to a server
-  # and get an incorrect response back.
-  #
-  # ref: https://bugs.ruby-lang.org/issues/2718
-  def self.close_drb_pool_connections
-    require 'drb'
-
-    # HACK: DRb doesn't provide an interface to close open pool connections.
-    #
-    # Once that is added this should be replaced.
-    DRb::DRbConn.instance_variable_get(:@mutex).synchronize do
-      DRb::DRbConn.instance_variable_get(:@pool).each(&:close)
-      DRb::DRbConn.instance_variable_set(:@pool, [])
-    end
-  end
 
   # Overriding queue_name as now some queue names can be
   # arrays of names for some workers not just a singular name.
@@ -383,39 +341,25 @@ def systemd_worker?
   end
 
   def start_runner
-    if ENV['MIQ_SPAWN_WORKERS'] || !Process.respond_to?(:fork)
-      start_runner_via_spawn
-    elsif systemd_worker?
+    if ENV['MIQ_SYSTEMD_WORKERS'] && systemd_worker?
       start_systemd_worker
     elsif containerized_worker?
       start_runner_via_container
     else
-      start_runner_via_fork
+      start_runner_via_spawn
     end
   end
 
   def start_runner_via_container
     create_container_objects
   end
 
-  def start_runner_via_fork
-    self.class.before_fork
-    pid = fork(:cow_friendly => true) do
-      self.class.after_fork
-      self.class::Runner.start_worker(worker_options)
-      exit!
-    end
-
-    Process.detach(pid)
-    pid
-  end
-
   def self.build_command_line(guid, ems_id = nil)
     raise ArgumentError, "No guid provided" unless guid
 
     require 'awesome_spawn'
     cmd = "#{Gem.ruby} #{runner_script}"
-    cmd = "nice #{nice_increment} #{cmd}" if ENV["APPLIANCE"]
+    cmd = "nice -n #{nice_increment} #{cmd}" if ENV["APPLIANCE"]
 
     options = {:guid => guid, :heartbeat => nil}
     if ems_id
@@ -645,13 +589,9 @@ def self.normalized_type
                          end
   end
 
-  def self.renice(pid)
-    AwesomeSpawn.run("renice", :params =>  {:n => nice_increment, :p => pid })
-  end
-
   def self.nice_increment
     delta = worker_settings[:nice_delta]
-    delta.kind_of?(Integer) ? delta.to_s : "+10"
+    delta.kind_of?(Integer) ? delta.to_s : "10"
   end
 
   def self.display_name(number = 1)

lib/extensions/ar_application_name.rb

@@ -4,10 +4,6 @@ module ArApplicationName
   # connections in the pool.  We do this because reconnect! on an instance of the
   # AR adapter created prior to our change to PGAPPNAME will have old connection
   # options, which will reset our application_name.
-  #
-  # Because we fork workers from the server, if we don't disconnect the pool,
-  # any call to reconnect! on a connection will cause the worker's connection
-  # to have the server's application_name.
   def self.name=(name)
     # TODO: this is postgresql specific
     ENV['PGAPPNAME'] = name

lib/workers/bin/run_single_worker.rb

@@ -89,7 +89,7 @@ def all_role_names
   exit 1
 end
 
-worker_class.before_fork
+worker_class.preload_for_worker_role if worker_class.respond_to?(:preload_for_worker_role)
 unless options[:dry_run]
   create_options = {:pid => Process.pid}
   runner_options = {}

spec/models/miq_worker_spec.rb

@@ -22,13 +22,6 @@ def all_workers
     end
   end
 
-  it "renice" do
-    allow(AwesomeSpawn).to receive(:launch)
-    allow(described_class).to receive(:worker_settings).and_return(:nice_delta => 5)
-    result = described_class.renice(123)
-    expect(result.command_line).to eq "renice -n 5 -p 123"
-  end
-
   context ".has_required_role?" do
     def check_has_required_role(worker_role_names, expected_result)
       allow(described_class).to receive(:required_roles).and_return(worker_role_names)
@@ -382,12 +375,12 @@ def check_has_required_role(worker_role_names, expected_result)
 
       it "with ENV['APPLIANCE']" do
         begin
-          allow(MiqWorker).to receive(:nice_increment).and_return("+10")
+          allow(MiqWorker).to receive(:nice_increment).and_return("10")
           allow(@worker).to receive(:worker_options).and_return(:ems_id => 1234, :guid => @worker.guid)
           old_env = ENV.delete('APPLIANCE')
           ENV['APPLIANCE'] = 'true'
           cmd = @worker.command_line
-          expect(cmd).to start_with("nice +10")
+          expect(cmd).to start_with("nice -n 10")
           expect(cmd).to include("--ems-id 1234")
           expect(cmd).to include("--guid #{@worker.guid}")
           expect(cmd).to include("--heartbeat")