Merge branch 'use-monotonic-time-in-sidekiq-daemon-memory-killer' into 'master'

Use monotonic time in Sidekiq daemon memory killer

See merge request gitlab-org/gitlab!17209

lib/gitlab/sidekiq_daemon/memory_killer.rb

@@ -17,6 +17,9 @@ module Gitlab
       CHECK_INTERVAL_SECONDS = [ENV.fetch('SIDEKIQ_MEMORY_KILLER_CHECK_INTERVAL', 3).to_i, 2].max
       # Give Sidekiq up to 30 seconds to allow existing jobs to finish after exceeding the limit
       SHUTDOWN_TIMEOUT_SECONDS = ENV.fetch('SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT', 30).to_i
+      # Developer/admin should always set `memory_killer_max_memory_growth_kb` explicitly
+      # In case not set, default to 300M. This is for extra-safe.
+      DEFAULT_MAX_MEMORY_GROWTH_KB = 300_000
 
       def initialize
         super
@@ -90,7 +93,7 @@ module Gitlab
 
       def rss_within_range?
         current_rss = nil
-        deadline = Time.now + GRACE_BALLOON_SECONDS.seconds
+        deadline = Gitlab::Metrics::System.monotonic_time + GRACE_BALLOON_SECONDS.seconds
         loop do
           return true unless enabled?
 
@@ -103,7 +106,7 @@ module Gitlab
           return true if current_rss < soft_limit_rss
 
           # RSS did not go below the soft limit within deadline, restart
-          break if Time.now > deadline
+          break if Gitlab::Metrics::System.monotonic_time > deadline
 
           sleep(CHECK_INTERVAL_SECONDS)
         end
@@ -159,11 +162,11 @@ module Gitlab
         )
         Process.kill(signal, pid)
 
-        deadline = Time.now + time
+        deadline = Gitlab::Metrics::System.monotonic_time + time
 
         # we try to finish as early as all jobs finished
         # so we retest that in loop
-        sleep(CHECK_INTERVAL_SECONDS) while enabled? && any_jobs? && Time.now < deadline
+        sleep(CHECK_INTERVAL_SECONDS) while enabled? && any_jobs? && Gitlab::Metrics::System.monotonic_time < deadline
       end
 
       def signal_pgroup(signal, explanation)
@@ -192,11 +195,11 @@ module Gitlab
 
       def rss_increase_by_job(job)
         memory_growth_kb = get_job_options(job, 'memory_killer_memory_growth_kb', 0).to_i
-        max_memory_growth_kb = get_job_options(job, 'memory_killer_max_memory_growth_kb', MAX_MEMORY_KB).to_i
+        max_memory_growth_kb = get_job_options(job, 'memory_killer_max_memory_growth_kb', DEFAULT_MAX_MEMORY_GROWTH_KB).to_i
 
         return 0 if memory_growth_kb.zero?
 
-        time_elapsed = Time.now.to_i - job[:started_at]
+        time_elapsed = [Gitlab::Metrics::System.monotonic_time - job[:started_at], 0].max
         [memory_growth_kb * time_elapsed, max_memory_growth_kb].min
       end
 

lib/gitlab/sidekiq_daemon/monitor.rb

@@ -26,7 +26,7 @@ module Gitlab
 
       def within_job(worker_class, jid, queue)
         jobs_mutex.synchronize do
-          jobs[jid] = { worker_class: worker_class, thread: Thread.current, started_at: Time.now.to_i }
+          jobs[jid] = { worker_class: worker_class, thread: Thread.current, started_at: Gitlab::Metrics::System.monotonic_time }
         end
 
         if cancelled?(jid)

spec/lib/gitlab/sidekiq_daemon/memory_killer_spec.rb

@@ -128,7 +128,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:soft_limit_rss).and_return(200)
       expect(memory_killer).to receive(:hard_limit_rss).and_return(300)
 
-      expect(Time).to receive(:now).and_call_original
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_call_original
       expect(memory_killer).not_to receive(:log_rss_out_of_range)
 
       expect(subject).to be true
@@ -139,7 +139,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:soft_limit_rss).at_least(:once).and_return(200)
       expect(memory_killer).to receive(:hard_limit_rss).at_least(:once).and_return(300)
 
-      expect(Time).to receive(:now).and_call_original
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_call_original
 
       expect(memory_killer).to receive(:log_rss_out_of_range).with(400, 300, 200)
 
@@ -151,7 +151,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:soft_limit_rss).at_least(:once).and_return(200)
       expect(memory_killer).to receive(:hard_limit_rss).at_least(:once).and_return(300)
 
-      expect(Time).to receive(:now).twice.and_call_original
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).twice.and_call_original
       expect(memory_killer).to receive(:sleep).with(check_interval_seconds)
 
       expect(memory_killer).to receive(:log_rss_out_of_range).with(400, 300, 200)
@@ -164,7 +164,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:soft_limit_rss).and_return(200, 200)
       expect(memory_killer).to receive(:hard_limit_rss).and_return(300, 300)
 
-      expect(Time).to receive(:now).twice.and_call_original
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).twice.and_call_original
       expect(memory_killer).to receive(:sleep).with(check_interval_seconds)
 
       expect(memory_killer).not_to receive(:log_rss_out_of_range)
@@ -177,7 +177,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:soft_limit_rss).exactly(5).times.and_return(200)
       expect(memory_killer).to receive(:hard_limit_rss).exactly(5).times.and_return(300)
 
-      expect(Time).to receive(:now).exactly(5).times.and_call_original
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).exactly(5).times.and_call_original
       expect(memory_killer).to receive(:sleep).exactly(3).times.with(check_interval_seconds).and_call_original
 
       expect(memory_killer).to receive(:log_rss_out_of_range).with(250, 300, 200)
@@ -219,7 +219,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
 
     it 'send signal and return when all jobs finished' do
       expect(Process).to receive(:kill).with(signal, pid).ordered
-      expect(Time).to receive(:now).and_call_original
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_call_original
 
       expect(memory_killer).to receive(:enabled?).and_return(true)
       expect(memory_killer).to receive(:any_jobs?).and_return(false)
@@ -231,7 +231,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
 
     it 'send signal and wait till deadline if any job not finished' do
       expect(Process).to receive(:kill).with(signal, pid).ordered
-      expect(Time).to receive(:now).and_call_original.at_least(:once)
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_call_original.at_least(:once)
 
       expect(memory_killer).to receive(:enabled?).and_return(true).at_least(:once)
       expect(memory_killer).to receive(:any_jobs?).and_return(true).at_least(:once)
@@ -351,7 +351,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
     subject { memory_killer.send(:rss_increase_by_job, job) }
 
     before do
-      stub_const("#{described_class}::MAX_MEMORY_KB", max_memory_kb)
+      stub_const("#{described_class}::DEFAULT_MAX_MEMORY_GROWTH_KB", max_memory_kb)
     end
 
     it 'return 0 if memory_growth_kb return 0' do
@@ -366,7 +366,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_memory_growth_kb', 0).and_return(10)
       expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_max_memory_growth_kb', max_memory_kb).and_return(100)
 
-      expect(Time).to receive(:now).and_return(323)
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(323)
       expect(subject).to eq(20)
     end
 
@@ -374,7 +374,7 @@ describe Gitlab::SidekiqDaemon::MemoryKiller do
       expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_memory_growth_kb', 0).and_return(10)
       expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_max_memory_growth_kb', max_memory_kb).and_return(100)
 
-      expect(Time).to receive(:now).and_return(332)
+      expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(332)
       expect(subject).to eq(100)
     end
   end