Automatic merge of gitlab-org/gitlab-ce master

app/models/ci/build.rb

@@ -121,6 +121,8 @@ module Ci
     scope :scheduled_actions, ->() { where(when: :delayed, status: COMPLETED_STATUSES + %i[scheduled]) }
     scope :ref_protected, -> { where(protected: true) }
     scope :with_live_trace, -> { where('EXISTS (?)', Ci::BuildTraceChunk.where('ci_builds.id = ci_build_trace_chunks.build_id').select(1)) }
+    scope :with_stale_live_trace, -> { with_live_trace.finished_before(12.hours.ago) }
+    scope :finished_before, -> (date) { finished.where('finished_at < ?', date) }
 
     scope :matches_tag_ids, -> (tag_ids) do
       matcher = ::ActsAsTaggableOn::Tagging

app/workers/ci/archive_traces_cron_worker.rb

@@ -10,7 +10,7 @@ module Ci
       # Archive stale live traces which still resides in redis or database
       # This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
       # More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
-      Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
+      Ci::Build.with_stale_live_trace.find_each(batch_size: 100) do |build|
         Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name)
       end
     end

changelogs/unreleased/avoid-race-condition-of-archive-trace-cron-worker.yml

+---
+title: Avoid conflicts between ArchiveTracesCronWorker and ArchiveTraceWorker
+merge_request: 31376
+author:
+type: fixed

package.json

@@ -39,7 +39,7 @@
     "@babel/preset-env": "^7.5.5",
     "@gitlab/csslab": "^1.9.0",
     "@gitlab/svgs": "^1.68.0",
-    "@gitlab/ui": "5.15.0",
+    "@gitlab/ui": "5.18.0",
     "apollo-cache-inmemory": "^1.5.1",
     "apollo-client": "^2.5.1",
     "apollo-link": "^1.2.11",

spec/models/ci/build_spec.rb

@@ -149,6 +149,56 @@ describe Ci::Build do
     end
   end
 
+  describe '.with_stale_live_trace' do
+    subject { described_class.with_stale_live_trace }
+
+    context 'when build has a stale live trace' do
+      let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.day.ago) }
+
+      it 'selects the build' do
+        is_expected.to eq([build])
+      end
+    end
+
+    context 'when build does not have a stale live trace' do
+      let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 1.hour.ago) }
+
+      it 'does not select the build' do
+        is_expected.to be_empty
+      end
+    end
+  end
+
+  describe '.finished_before' do
+    subject { described_class.finished_before(date) }
+
+    let(:date) { 1.hour.ago }
+
+    context 'when build has finished one day ago' do
+      let!(:build) { create(:ci_build, :success, finished_at: 1.day.ago) }
+
+      it 'selects the build' do
+        is_expected.to eq([build])
+      end
+    end
+
+    context 'when build has finished 30 minutes ago' do
+      let!(:build) { create(:ci_build, :success, finished_at: 30.minutes.ago) }
+
+      it 'returns an empty array' do
+        is_expected.to be_empty
+      end
+    end
+
+    context 'when build is still running' do
+      let!(:build) { create(:ci_build, :running) }
+
+      it 'returns an empty array' do
+        is_expected.to be_empty
+      end
+    end
+  end
+
   describe '.with_reports' do
     subject { described_class.with_reports(Ci::JobArtifact.test_reports) }
 

spec/workers/ci/archive_traces_cron_worker_spec.rb

@@ -5,6 +5,8 @@ require 'spec_helper'
 describe Ci::ArchiveTracesCronWorker do
   subject { described_class.new.perform }
 
+  let(:finished_at) { 1.day.ago }
+
   before do
     stub_feature_flags(ci_enable_live_trace: true)
   end
@@ -28,7 +30,7 @@ describe Ci::ArchiveTracesCronWorker do
   end
 
   context 'when a job succeeded' do
-    let!(:build) { create(:ci_build, :success, :trace_live) }
+    let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) }
 
     it_behaves_like 'archives trace'
 
@@ -39,9 +41,15 @@ describe Ci::ArchiveTracesCronWorker do
       subject
     end
 
+    context 'when the job finished recently' do
+      let(:finished_at) { 1.hour.ago }
+
+      it_behaves_like 'does not archive trace'
+    end
+
     context 'when a trace had already been archived' do
       let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) }
-      let!(:build2) { create(:ci_build, :success, :trace_live) }
+      let!(:build2) { create(:ci_build, :success, :trace_live, finished_at: finished_at) }
 
       it 'continues to archive live traces' do
         subject
@@ -52,7 +60,7 @@ describe Ci::ArchiveTracesCronWorker do
     end
 
     context 'when an unexpected exception happened during archiving' do
-      let!(:build) { create(:ci_build, :success, :trace_live) }
+      let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) }
 
       before do
         allow(Gitlab::Sentry).to receive(:track_exception)
@@ -71,7 +79,7 @@ describe Ci::ArchiveTracesCronWorker do
   end
 
   context 'when a job was cancelled' do
-    let!(:build) { create(:ci_build, :canceled, :trace_live) }
+    let!(:build) { create(:ci_build, :canceled, :trace_live, finished_at: finished_at) }
 
     it_behaves_like 'archives trace'
   end

yarn.lock

@@ -1003,10 +1003,10 @@
   resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.68.0.tgz#d631bd84ea7907592240d8417e82ba66d6a54c0c"
   integrity sha512-3JmIq0bHg4InjLooM+kQFPfg3d7B1Pye67pN9+12kZXIa0nRGuwKEq3WSbcS+ACdg5jcVPNPYqStItEO4teHdw==
 
-"@gitlab/ui@5.15.0":
-  version "5.15.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.15.0.tgz#1ce92cfed77dcd63a90d751043b42b19e64431c9"
-  integrity sha512-YrYgERcmTxC+oP4GaY7onqvYgvTsyGCiiegQbZbXdNRLGGAmvfxWPzQRz8Ci9yIYkLvi0X2AV7BT8RTVOPQgXg==
+"@gitlab/ui@5.18.0":
+  version "5.18.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.18.0.tgz#f7f93ad41673f5ae081cd2246dda5456d3c956a2"
+  integrity sha512-umB7JCKVDZOajed0N563JBYSpiyK+VDL2eCdkinW+twWhMct8onoW4CLTpkgBi6Z9Y1Bq47aqnFtGf+1IKNIGw==
   dependencies:
     "@babel/standalone" "^7.0.0"
     "@gitlab/vue-toasted" "^1.2.1"