Send SIGSTP before SIGTERM to actually give Sidekiq jobs 30s to finish when the memory killer kicks in

1 files changed, 27 insertions, 14 deletions

diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb
index d7d24eeb37b..2bfb7caefd9 100644
--- a/lib/gitlab/sidekiq_middleware/memory_killer.rb
+++ b/lib/gitlab/sidekiq_middleware/memory_killer.rb
@@ -7,7 +7,6 @@ module Gitlab
       GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i
       # Wait 30 seconds for running jobs to finish during graceful shutdown
       SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i
-      SHUTDOWN_SIGNAL = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_SIGNAL'] || 'SIGKILL').to_s
 
       # Create a mutex used to ensure there will be only one thread waiting to
       # shut Sidekiq down
@@ -15,6 +14,7 @@ module Gitlab
 
       def call(worker, job, queue)
         yield
+
         current_rss = get_rss
 
         return unless MAX_RSS > 0 && current_rss > MAX_RSS
@@ -23,32 +23,45 @@ module Gitlab
           # Return if another thread is already waiting to shut Sidekiq down
           return unless MUTEX.try_lock
 
-          Sidekiq.logger.warn "current RSS #{current_rss} exceeds maximum RSS "\
-            "#{MAX_RSS}"
-          Sidekiq.logger.warn "this thread will shut down PID #{Process.pid} - Worker #{worker.class} - JID-#{job['jid']} "\
-            "in #{GRACE_TIME} seconds"
-          sleep(GRACE_TIME)
+          Sidekiq.logger.warn "Sidekiq worker PID-#{pid} current RSS #{current_rss}"\
+            " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}"
+          Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later"
 
-          Sidekiq.logger.warn "sending SIGTERM to PID #{Process.pid} - Worker #{worker.class} - JID-#{job['jid']}"
-          Process.kill('SIGTERM', Process.pid)
+          # Wait `GRACE_TIME` to give the memory intensive job time to finish.
+          # Then, tell Sidekiq to stop fetching new jobs.
+          wait_and_signal(GRACE_TIME, 'SIGSTP', 'stop fetching new jobs')
 
-          Sidekiq.logger.warn "waiting #{SHUTDOWN_WAIT} seconds before sending "\
-            "#{SHUTDOWN_SIGNAL} to PID #{Process.pid} - Worker #{worker.class} - JID-#{job['jid']}"
-          sleep(SHUTDOWN_WAIT)
+          # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish.
+          # Then, tell Sidekiq to gracefully shut down by giving jobs a few more
+          # moments to finish, killing and requeuing them if they didn't, and
+          # then terminating itself.
+          wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down')
 
-          Sidekiq.logger.warn "sending #{SHUTDOWN_SIGNAL} to PID #{Process.pid} - Worker #{worker.class} - JID-#{job['jid']}"
-          Process.kill(SHUTDOWN_SIGNAL, Process.pid)
+          # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't.
+          wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die')
         end
       end
 
       private
 
       def get_rss
-        output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{Process.pid}))
+        output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}))
         return 0 unless status.zero?
 
         output.to_i
       end
+
+      def wait_and_signal(time, signal, explanation)
+        Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
+        sleep(time)
+
+        Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
+        Process.kill(signal, pid)
+      end
+
+      def pid
+        Process.pid
+      end
     end
   end
 end