Add latest changes from gitlab-org/gitlab@13-1-stable-ee

5 files changed, 223 insertions, 43 deletions

diff --git a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb
index 5f80ef9538a..1d45b70ec3e 100644
--- a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb
@@ -47,8 +47,11 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
     end
 
     context "when workers are not attributed" do
-      class TestNonAttributedWorker
-        include Sidekiq::Worker
+      before do
+        stub_const('TestNonAttributedWorker', Class.new)
+        TestNonAttributedWorker.class_eval do
+          include Sidekiq::Worker
+        end
       end
 
       it_behaves_like "a metrics client middleware" do
diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb
index 9c7f6638913..a1e4cbb1e31 100644
--- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb
@@ -31,14 +31,51 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::Client, :clean_gitlab_redis_q
       expect(job3['duplicate-of']).to eq(job1['jid'])
     end
 
-    it "does not mark a job that's scheduled in the future as a duplicate" do
-      TestDeduplicationWorker.perform_async('args1')
-      TestDeduplicationWorker.perform_at(1.day.from_now, 'args1')
-      TestDeduplicationWorker.perform_in(3.hours, 'args1')
+    context 'without scheduled deduplication' do
+      it "does not mark a job that's scheduled in the future as a duplicate" do
+        TestDeduplicationWorker.perform_async('args1')
+        TestDeduplicationWorker.perform_at(1.day.from_now, 'args1')
+        TestDeduplicationWorker.perform_in(3.hours, 'args1')
 
-      duplicates = TestDeduplicationWorker.jobs.map { |job| job['duplicate-of'] }
+        duplicates = TestDeduplicationWorker.jobs.map { |job| job['duplicate-of'] }
 
-      expect(duplicates).to all(be_nil)
+        expect(duplicates).to all(be_nil)
+      end
+    end
+
+    context 'with scheduled deduplication' do
+      let(:scheduled_worker_class) do
+        Class.new do
+          def self.name
+            'TestDeduplicationWorker'
+          end
+
+          include ApplicationWorker
+
+          deduplicate :until_executing, including_scheduled: true
+
+          def perform(*args)
+          end
+        end
+      end
+
+      before do
+        stub_const('TestDeduplicationWorker', scheduled_worker_class)
+      end
+
+      it 'adds a correct duplicate tag to the jobs', :aggregate_failures do
+        TestDeduplicationWorker.perform_async('args1')
+        TestDeduplicationWorker.perform_at(1.day.from_now, 'args1')
+        TestDeduplicationWorker.perform_in(3.hours, 'args1')
+        TestDeduplicationWorker.perform_in(3.hours, 'args2')
+
+        job1, job2, job3, job4 = TestDeduplicationWorker.jobs
+
+        expect(job1['duplicate-of']).to be_nil
+        expect(job2['duplicate-of']).to eq(job1['jid'])
+        expect(job3['duplicate-of']).to eq(job1['jid'])
+        expect(job4['duplicate-of']).to be_nil
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb
index 929df0a7ffb..13c86563be7 100644
--- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb
@@ -93,6 +93,25 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r
     end
   end
 
+  describe '#scheduled?' do
+    it 'returns false for non-scheduled jobs' do
+      expect(duplicate_job.scheduled?).to be(false)
+    end
+
+    context 'scheduled jobs' do
+      let(:job) do
+        { 'class' => 'AuthorizedProjectsWorker',
+          'args' => [1],
+          'jid' => '123',
+          'at' => 42 }
+      end
+
+      it 'returns true' do
+        expect(duplicate_job.scheduled?).to be(true)
+      end
+    end
+  end
+
   describe '#duplicate?' do
     it "raises an error if the check wasn't performed" do
       expect { duplicate_job.duplicate? }.to raise_error /Call `#check!` first/
@@ -112,28 +131,23 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r
     end
   end
 
-  describe 'droppable?' do
-    where(:idempotent, :duplicate, :prevent_deduplication) do
-      # [true, false].repeated_permutation(3)
-      [[true, true, true],
-       [true, true, false],
-       [true, false, true],
-       [true, false, false],
-       [false, true, true],
-       [false, true, false],
-       [false, false, true],
-       [false, false, false]]
+  describe '#droppable?' do
+    where(:idempotent, :prevent_deduplication) do
+      # [true, false].repeated_permutation(2)
+      [[true, true],
+       [true, false],
+       [false, true],
+       [false, false]]
     end
 
     with_them do
       before do
         allow(AuthorizedProjectsWorker).to receive(:idempotent?).and_return(idempotent)
-        allow(duplicate_job).to receive(:duplicate?).and_return(duplicate)
         stub_feature_flags("disable_#{queue}_deduplication" => prevent_deduplication)
       end
 
       it 'is droppable when all conditions are met' do
-        if idempotent && duplicate && !prevent_deduplication
+        if idempotent && !prevent_deduplication
           expect(duplicate_job).to be_droppable
         else
           expect(duplicate_job).not_to be_droppable
@@ -142,6 +156,31 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r
     end
   end
 
+  describe '#scheduled_at' do
+    let(:scheduled_at) { 42 }
+    let(:job) do
+      { 'class' => 'AuthorizedProjectsWorker',
+        'args' => [1],
+        'jid' => '123',
+        'at' => scheduled_at }
+    end
+
+    it 'returns when the job is scheduled at' do
+      expect(duplicate_job.scheduled_at).to eq(scheduled_at)
+    end
+  end
+
+  describe '#options' do
+    let(:worker_options) { { foo: true } }
+
+    it 'returns worker options' do
+      allow(AuthorizedProjectsWorker).to(
+        receive(:get_deduplication_options).and_return(worker_options))
+
+      expect(duplicate_job.options).to eq(worker_options)
+    end
+  end
+
   def set_idempotency_key(key, value = '1')
     Sidekiq.redis { |r| r.set(key, value) }
   end
diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb
index 31b51260ebd..eb8b0a951a8 100644
--- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'fast_spec_helper'
+require 'timecop'
 
 describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do
   let(:fake_duplicate_job) do
@@ -15,28 +16,90 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do
     end
 
     it 'checks for duplicates before yielding' do
-      expect(fake_duplicate_job).to receive(:check!).ordered.and_return('a jid')
+      expect(fake_duplicate_job).to receive(:scheduled?).twice.ordered.and_return(false)
+      expect(fake_duplicate_job).to(
+        receive(:check!)
+          .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL)
+          .ordered
+          .and_return('a jid'))
       expect(fake_duplicate_job).to receive(:duplicate?).ordered.and_return(false)
-      expect(fake_duplicate_job).to receive(:droppable?).ordered.and_return(false)
 
       expect { |b| strategy.schedule({}, &b) }.to yield_control
     end
 
-    it 'adds the jid of the existing job to the job hash' do
-      allow(fake_duplicate_job).to receive(:check!).and_return('the jid')
-      allow(fake_duplicate_job).to receive(:droppable?).and_return(true)
-      job_hash = {}
+    it 'checks worker options for scheduled jobs' do
+      expect(fake_duplicate_job).to receive(:scheduled?).ordered.and_return(true)
+      expect(fake_duplicate_job).to receive(:options).ordered.and_return({})
+      expect(fake_duplicate_job).not_to receive(:check!)
 
-      expect(fake_duplicate_job).to receive(:duplicate?).and_return(true)
-      expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid')
+      expect { |b| strategy.schedule({}, &b) }.to yield_control
+    end
+
+    context 'job marking' do
+      it 'adds the jid of the existing job to the job hash' do
+        allow(fake_duplicate_job).to receive(:scheduled?).and_return(false)
+        allow(fake_duplicate_job).to receive(:check!).and_return('the jid')
+        allow(fake_duplicate_job).to receive(:droppable?).and_return(true)
+        job_hash = {}
 
-      strategy.schedule(job_hash) {}
+        expect(fake_duplicate_job).to receive(:duplicate?).and_return(true)
+        expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid')
 
-      expect(job_hash).to include('duplicate-of' => 'the jid')
+        strategy.schedule(job_hash) {}
+
+        expect(job_hash).to include('duplicate-of' => 'the jid')
+      end
+
+      context 'scheduled jobs' do
+        let(:time_diff) { 1.minute }
+
+        context 'scheduled in the past' do
+          it 'adds the jid of the existing job to the job hash' do
+            allow(fake_duplicate_job).to receive(:scheduled?).twice.and_return(true)
+            allow(fake_duplicate_job).to receive(:scheduled_at).and_return(Time.now - time_diff)
+            allow(fake_duplicate_job).to receive(:options).and_return({ including_scheduled: true })
+            allow(fake_duplicate_job).to(
+              receive(:check!)
+                .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL)
+                .and_return('the jid'))
+            allow(fake_duplicate_job).to receive(:droppable?).and_return(true)
+            job_hash = {}
+
+            expect(fake_duplicate_job).to receive(:duplicate?).and_return(true)
+            expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid')
+
+            strategy.schedule(job_hash) {}
+
+            expect(job_hash).to include('duplicate-of' => 'the jid')
+          end
+        end
+
+        context 'scheduled in the future' do
+          it 'adds the jid of the existing job to the job hash' do
+            Timecop.freeze do
+              allow(fake_duplicate_job).to receive(:scheduled?).twice.and_return(true)
+              allow(fake_duplicate_job).to receive(:scheduled_at).and_return(Time.now + time_diff)
+              allow(fake_duplicate_job).to receive(:options).and_return({ including_scheduled: true })
+              allow(fake_duplicate_job).to(
+                receive(:check!).with(time_diff.to_i).and_return('the jid'))
+              allow(fake_duplicate_job).to receive(:droppable?).and_return(true)
+              job_hash = {}
+
+              expect(fake_duplicate_job).to receive(:duplicate?).and_return(true)
+              expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid')
+
+              strategy.schedule(job_hash) {}
+
+              expect(job_hash).to include('duplicate-of' => 'the jid')
+            end
+          end
+        end
+      end
     end
 
     context "when the job is droppable" do
       before do
+        allow(fake_duplicate_job).to receive(:scheduled?).and_return(false)
         allow(fake_duplicate_job).to receive(:check!).and_return('the jid')
         allow(fake_duplicate_job).to receive(:duplicate?).and_return(true)
         allow(fake_duplicate_job).to receive(:existing_jid).and_return('the jid')
@@ -52,7 +115,7 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do
         expect(schedule_result).to be(false)
       end
 
-      it 'logs that the job wass dropped' do
+      it 'logs that the job was dropped' do
         fake_logger = instance_double(Gitlab::SidekiqLogging::DeduplicationLogger)
 
         expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger)
diff --git a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb
index 3214bd758e7..4b7baea25e8 100644
--- a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb
@@ -31,7 +31,11 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
         let(:gitaly_seconds_metric) { double('gitaly seconds metric') }
         let(:failed_total_metric) { double('failed total metric') }
         let(:retried_total_metric) { double('retried total metric') }
+        let(:redis_requests_total) { double('redis calls total metric') }
         let(:running_jobs_metric) { double('running jobs metric') }
+        let(:redis_seconds_metric) { double('redis seconds metric') }
+        let(:elasticsearch_seconds_metric) { double('elasticsearch seconds metric') }
+        let(:elasticsearch_requests_total) { double('elasticsearch calls total metric') }
 
         before do
           allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_queue_duration_seconds, anything, anything, anything).and_return(queue_duration_seconds)
@@ -39,8 +43,12 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
           allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_cpu_seconds, anything, anything, anything).and_return(user_execution_seconds_metric)
           allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_db_seconds, anything, anything, anything).and_return(db_seconds_metric)
           allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_gitaly_seconds, anything, anything, anything).and_return(gitaly_seconds_metric)
+          allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_redis_requests_duration_seconds, anything, anything, anything).and_return(redis_seconds_metric)
+          allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_elasticsearch_requests_duration_seconds, anything, anything, anything).and_return(elasticsearch_seconds_metric)
           allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_failed_total, anything).and_return(failed_total_metric)
           allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_retried_total, anything).and_return(retried_total_metric)
+          allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_redis_requests_total, anything).and_return(redis_requests_total)
+          allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_elasticsearch_requests_total, anything).and_return(elasticsearch_requests_total)
           allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_running_jobs, anything, {}, :all).and_return(running_jobs_metric)
           allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_concurrency, anything, {}, :all).and_return(concurrency_metric)
 
@@ -69,21 +77,35 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
           let(:db_duration) { 3 }
           let(:gitaly_duration) { 4 }
 
+          let(:redis_calls) { 2 }
+          let(:redis_duration) { 0.01 }
+
+          let(:elasticsearch_calls) { 8 }
+          let(:elasticsearch_duration) { 0.54 }
+
           before do
             allow(subject).to receive(:get_thread_cputime).and_return(thread_cputime_before, thread_cputime_after)
             allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after)
             allow(Gitlab::InstrumentationHelper).to receive(:queue_duration_for_job).with(job).and_return(queue_duration_for_job)
             allow(ActiveRecord::LogSubscriber).to receive(:runtime).and_return(db_duration * 1000)
-            allow(subject).to receive(:get_gitaly_time).and_return(gitaly_duration)
-
-            expect(running_jobs_metric).to receive(:increment).with(labels, 1)
-            expect(running_jobs_metric).to receive(:increment).with(labels, -1)
 
-            expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job
-            expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration)
-            expect(db_seconds_metric).to receive(:observe).with(labels_with_job_status, db_duration)
-            expect(gitaly_seconds_metric).to receive(:observe).with(labels_with_job_status, gitaly_duration)
-            expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration)
+            job[:gitaly_duration_s] = gitaly_duration
+            job[:redis_calls] = redis_calls
+            job[:redis_duration_s] = redis_duration
+
+            job[:elasticsearch_calls] = elasticsearch_calls
+            job[:elasticsearch_duration_s] = elasticsearch_duration
+
+            allow(running_jobs_metric).to receive(:increment)
+            allow(redis_requests_total).to receive(:increment)
+            allow(elasticsearch_requests_total).to receive(:increment)
+            allow(queue_duration_seconds).to receive(:observe)
+            allow(user_execution_seconds_metric).to receive(:observe)
+            allow(db_seconds_metric).to receive(:observe)
+            allow(gitaly_seconds_metric).to receive(:observe)
+            allow(completion_seconds_metric).to receive(:observe)
+            allow(redis_seconds_metric).to receive(:observe)
+            allow(elasticsearch_seconds_metric).to receive(:observe)
           end
 
           it 'yields block' do
@@ -91,6 +113,18 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
           end
 
           it 'sets queue specific metrics' do
+            expect(running_jobs_metric).to receive(:increment).with(labels, -1)
+            expect(running_jobs_metric).to receive(:increment).with(labels, 1)
+            expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job
+            expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration)
+            expect(db_seconds_metric).to receive(:observe).with(labels_with_job_status, db_duration)
+            expect(gitaly_seconds_metric).to receive(:observe).with(labels_with_job_status, gitaly_duration)
+            expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration)
+            expect(redis_seconds_metric).to receive(:observe).with(labels_with_job_status, redis_duration)
+            expect(elasticsearch_seconds_metric).to receive(:observe).with(labels_with_job_status, elasticsearch_duration)
+            expect(redis_requests_total).to receive(:increment).with(labels_with_job_status, redis_calls)
+            expect(elasticsearch_requests_total).to receive(:increment).with(labels_with_job_status, elasticsearch_calls)
+
             subject.call(worker, job, :test) { nil }
           end
 
@@ -144,9 +178,13 @@ describe Gitlab::SidekiqMiddleware::ServerMetrics do
     end
 
     context "when workers are not attributed" do
-      class TestNonAttributedWorker
-        include Sidekiq::Worker
+      before do
+        stub_const('TestNonAttributedWorker', Class.new)
+        TestNonAttributedWorker.class_eval do
+          include Sidekiq::Worker
+        end
       end
+
       let(:worker) { TestNonAttributedWorker.new }
       let(:labels) { default_labels.merge(urgency: "") }