Merge branch 'osw-improve-stuck-sidekiq-processes-termination' into 'master'

Improve Sidekiq process hard shutdown (Cluster mode)

See merge request gitlab-org/gitlab!28734

config/initializers/sidekiq_cluster.rb

@@ -13,6 +13,16 @@ if ENV['ENABLE_SIDEKIQ_CLUSTER']
       # this case the parent PID changes and we need to terminate ourselves.
       if Process.ppid != parent
         Process.kill(:TERM, Process.pid)
+
+        # Wait for just a few extra seconds for a final attempt to
+        # gracefully terminate. Considering the parent (cluster) process
+        # have changed (SIGKILL'd), it shouldn't take long to shutdown.
+        sleep(5)
+
+        # Signaling the Sidekiq Pgroup as KILL is not forwarded to
+        # a possible child process. In Sidekiq Cluster, all child Sidekiq
+        # processes are PGROUP leaders (each process has its own pgroup).
+        Process.kill(:KILL, 0)
         break
       end
     end

lib/gitlab/sidekiq_cluster/cli.rb

@@ -115,7 +115,7 @@ module Gitlab
       end
 
       def hard_stop_stuck_pids
-        SidekiqCluster.signal_processes(SidekiqCluster.pids_alive(@processes), :KILL)
+        SidekiqCluster.signal_processes(SidekiqCluster.pids_alive(@processes), "-KILL")
       end
 
       def wait_for_termination

spec/lib/gitlab/sidekiq_cluster/cli_spec.rb

@@ -236,7 +236,7 @@ describe Gitlab::SidekiqCluster::CLI do
         .with([]).and_return([])
 
       expect(Gitlab::SidekiqCluster).to receive(:signal_processes)
-        .with([], :KILL)
+        .with([], "-KILL")
 
       stub_const("Gitlab::SidekiqCluster::CLI::CHECK_TERMINATE_INTERVAL_SECONDS", 0.1)
       allow(cli).to receive(:terminate_timeout_seconds) { 1 }
@@ -264,7 +264,7 @@ describe Gitlab::SidekiqCluster::CLI do
           .with(worker_pids).and_return([102])
 
         expect(Gitlab::SidekiqCluster).to receive(:signal_processes)
-          .with([102], :KILL)
+          .with([102], "-KILL")
 
         cli.run(%w(foo))
 

spec/lib/gitlab/sidekiq_cluster_spec.rb

@@ -44,7 +44,7 @@ describe Gitlab::SidekiqCluster do
   end
 
   describe '.signal_processes' do
-    it 'sends a signal to every thread' do
+    it 'sends a signal to every given process' do
       expect(described_class).to receive(:signal).with(1, :INT)
 
       described_class.signal_processes([1], :INT)