From 4064481501a24d31872914c845f5d8c2cfc08040 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Fri, 1 Jun 2018 16:25:17 +0900
Subject: [PATCH] Rename find_stale_in_batches to
 find_builds_from_stale_live_trace.  Fix comments

---
 app/models/ci/build_trace_chunk.rb               |  4 ++--
 app/workers/ci/rescue_stale_live_trace_worker.rb | 10 ++++------
 spec/models/ci/build_trace_chunk_spec.rb         | 12 ++++++------
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 7940f7c2ee7..80a63434283 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -52,13 +52,13 @@ module Ci
       end
 
       # Find stale live traces and return their build ids
-      def find_stale_in_batches(finished_before: 1.hour.ago)
+      def find_builds_from_stale_live_trace
         include(EachBatch)
           .select(:build_id)
           .group(:build_id)
           .joins(:build)
           .merge(Ci::Build.finished)
-          .where('ci_builds.finished_at < ?', finished_before)
+          .where('ci_builds.finished_at < ?', 1.hour.ago)
           .each_batch(column: :build_id) do |chunks|
             build_ids = chunks.map { |chunk| chunk.build_id }
 
diff --git a/app/workers/ci/rescue_stale_live_trace_worker.rb b/app/workers/ci/rescue_stale_live_trace_worker.rb
index be0194b63ae..c4a462446d4 100644
--- a/app/workers/ci/rescue_stale_live_trace_worker.rb
+++ b/app/workers/ci/rescue_stale_live_trace_worker.rb
@@ -4,12 +4,10 @@ module Ci
     include CronjobQueue
 
     def perform
-      # Reschedule to archive live traces
-      #
-      # The targets are jobs with the following conditions
-      # - Jobs had been finished 1 hour ago, but they don't have an archived trace yet
-      #   This could happen when their sidekiq-jobs are lost by SIGKILL
-      Ci::BuildTraceChunk.find_stale_in_batches(finished_before: 1.hour.ago) do |build_ids|
+      # Archive live traces which still resides in redis or database
+      # This could happen when sidekiq-jobs for archivements are lost by SIGKILL
+      # Issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
+      Ci::BuildTraceChunk.find_builds_from_stale_live_trace do |build_ids|
         Ci::Build.where(id: build_ids).find_each do |build|
           begin
             build.trace.archive!
diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb
index 246152d1c8f..5d524d7cf49 100644
--- a/spec/models/ci/build_trace_chunk_spec.rb
+++ b/spec/models/ci/build_trace_chunk_spec.rb
@@ -35,8 +35,8 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
     end
   end
 
-  describe '.find_stale_in_batches' do
-    subject { described_class.find_stale_in_batches }
+  describe '.find_builds_from_stale_live_trace' do
+    subject { described_class.find_builds_from_stale_live_trace }
 
     context 'when build status is finished' do
       context 'when build finished 2 days ago' do
@@ -44,7 +44,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
           let!(:build) { create(:ci_build, :success, :trace_artifact, finished_at: 2.days.ago) }
 
           it 'does not yield build id' do
-            expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control
+            expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control
           end
         end
 
@@ -52,7 +52,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
           let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 2.days.ago) }
 
           it 'yields build id' do
-            expect { |b| described_class.find_stale_in_batches(&b) }.to yield_with_args([build.id])
+            expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.to yield_with_args([build.id])
           end
         end
       end
@@ -61,7 +61,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
         let!(:build) { create(:ci_build, :success, :trace_live, finished_at: 10.minutes.ago) }
 
         it 'does not yield build id' do
-          expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control
+          expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control
         end
       end
     end
@@ -70,7 +70,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
       let!(:build) { create(:ci_build, :running, :trace_live) }
 
       it 'does not yield build id' do
-        expect { |b| described_class.find_stale_in_batches(&b) }.not_to yield_control
+        expect { |b| described_class.find_builds_from_stale_live_trace(&b) }.not_to yield_control
       end
     end
   end