Merge branch 'sidekiq_memory_killer_grace_time' into 'master'

Add 'grace time' behavior to Sidekiq MemoryKiller

The previous iteration of the memory killer did not give 'large'
jobs much of a chance to finish. This caused them to come back after
each restart, leading to another memory-induced shutdown. By adding
some 'grace time' between the moment we detect a memory peak and
the moment we start shutting down, we hope to give Sidekiq a chance
to get whatever causes its memory to grow 'out of its system'.

See merge request !1313

2 files changed, 30 insertions, 25 deletions

diff --git a/config/initializers/4_sidekiq.rb b/config/initializers/4_sidekiq.rb
index 75c543c0f47..e856499732e 100644
--- a/config/initializers/4_sidekiq.rb
+++ b/config/initializers/4_sidekiq.rb
@@ -15,7 +15,7 @@ Sidekiq.configure_server do |config|
 
   config.server_middleware do |chain|
     chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS']
-    chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MAX_RSS']
+    chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS']
   end
 end
 
diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb
index 0fb09d3f228..0f2db50e98c 100644
--- a/lib/gitlab/sidekiq_middleware/memory_killer.rb
+++ b/lib/gitlab/sidekiq_middleware/memory_killer.rb
@@ -1,26 +1,41 @@
 module Gitlab
   module SidekiqMiddleware
     class MemoryKiller
+      # Default the RSS limit to 0, meaning the MemoryKiller is disabled
+      MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i
+      # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit
+      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
-      GRACEFUL_SHUTDOWN_WAIT = 30
+      SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i
+
+      # Create a mutex used to ensure there will be only one thread waiting to
+      # shut Sidekiq down
+      MUTEX = Mutex.new
 
       def call(worker, job, queue)
         yield
         current_rss = get_rss
-        return unless max_rss > 0 && current_rss > max_rss
-
-        Sidekiq.logger.warn "current RSS #{current_rss} exceeds maximum RSS "\
-         "#{max_rss}"
-        Sidekiq.logger.warn "sending SIGUSR1 to PID #{Process.pid}"
-        # SIGUSR1 tells Sidekiq to stop accepting new jobs
-        Process.kill('SIGUSR1', Process.pid)
-
-        Sidekiq.logger.warn "spawning thread that will send SIGTERM to PID "\
-          "#{Process.pid} in #{graceful_shutdown_wait} seconds"
-        # Send the final shutdown signal to Sidekiq from a separate thread so
-        # that the current job can finish
+
+        return unless MAX_RSS > 0 && current_rss > MAX_RSS
+
         Thread.new do
-          sleep(graceful_shutdown_wait)
+          # 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 "spawned thread that will shut down PID "\
+            "#{Process.pid} in #{GRACE_TIME} seconds"
+          sleep(GRACE_TIME)
+
+          Sidekiq.logger.warn "sending SIGUSR1 to PID #{Process.pid}"
+          Process.kill('SIGUSR1', Process.pid)
+
+          Sidekiq.logger.warn "waiting #{SHUTDOWN_WAIT} seconds before sending "\
+            "SIGTERM to PID #{Process.pid}"
+          sleep(SHUTDOWN_WAIT)
+
+          Sidekiq.logger.warn "sending SIGTERM to PID #{Process.pid}"
           Process.kill('SIGTERM', Process.pid)
         end
       end
@@ -33,16 +48,6 @@ module Gitlab
 
         output.to_i
       end
-
-      def max_rss
-        @max_rss ||= ENV['SIDEKIQ_MAX_RSS'].to_s.to_i
-      end
-
-      def graceful_shutdown_wait
-        @graceful_shutdown_wait ||= (
-          ENV['SIDEKIQ_GRACEFUL_SHUTDOWN_WAIT'] || GRACEFUL_SHUTDOWN_WAIT
-        ).to_i
-      end
     end
   end
 end