From 04ab732f30c689d87dee0e1b8c5832a0b6824269 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Tue, 3 Jul 2018 14:00:25 +0900
Subject: [PATCH] Remove traces efficiently

---
 app/models/ci/build.rb |  6 ++++++
 lib/gitlab/ci/trace.rb | 19 ++++++++++++-------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 41446946a5e..1a2292b4a20 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -376,6 +376,10 @@ module Ci
       trace.exist?
     end
 
+    def has_old_trace?
+      old_trace.present?
+    end
+
     def trace=(data)
       raise NotImplementedError
     end
@@ -385,6 +389,8 @@ module Ci
     end
 
     def erase_old_trace!
+      raise Gitlab::Ci::Trace::EraseError, 'Old trace does not exist' unless has_old_trace?
+
       update_column(:trace, nil)
     end
 
diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb
index a52d71225bb..ddf78b0b893 100644
--- a/lib/gitlab/ci/trace.rb
+++ b/lib/gitlab/ci/trace.rb
@@ -6,6 +6,7 @@ module Gitlab
       LEASE_TIMEOUT = 1.hour
 
       ArchiveError = Class.new(StandardError)
+      EraseError = Class.new(StandardError)
 
       attr_reader :job
 
@@ -98,14 +99,18 @@ module Gitlab
       end
 
       def erase!
-        trace_artifact&.destroy
+        ##
+        # Erase an archived traces
+        # This removes both a database-row and a real file in either a file storage or a object storage
+        trace_artifact&.destroy!
 
-        paths.each do |trace_path|
-          FileUtils.rm(trace_path, force: true)
-        end
-
-        job.trace_chunks.fast_destroy_all
-        job.erase_old_trace!
+        ##
+        # Erase a live trace
+        # Basically, jobs have _one_ of the following live traces, but it might be able to happen by a race condition
+        # Therefore, we remove all type of live traces.
+        job.trace_chunks.fast_destroy_all # Destroy chunks of a live trace
+        FileUtils.rm_f(current_path) if current_path # Remove a trace file of a live trace
+        job.erase_old_trace! if job.has_old_trace? # Remove a trace in database of a live trace
       end
 
       def archive!