From 644529590a263f8db215d288c2f59abbe632a09b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Thu, 7 Jun 2018 10:04:55 +0200
Subject: Allow to store BuildTraceChunks on Object Storage

---
 app/models/ci/build_trace_chunk.rb           | 105 ++++++++++++---------------
 app/models/ci/build_trace_chunks/database.rb |  29 ++++++++
 app/models/ci/build_trace_chunks/fog.rb      |  59 +++++++++++++++
 app/models/ci/build_trace_chunks/redis.rb    |  51 +++++++++++++
 4 files changed, 186 insertions(+), 58 deletions(-)
 create mode 100644 app/models/ci/build_trace_chunks/database.rb
 create mode 100644 app/models/ci/build_trace_chunks/fog.rb
 create mode 100644 app/models/ci/build_trace_chunks/redis.rb

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 4856f10846c..6b89efe8eb0 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -10,45 +10,48 @@ module Ci
     WriteError = Class.new(StandardError)
 
     CHUNK_SIZE = 128.kilobytes
-    CHUNK_REDIS_TTL = 1.week
     WRITE_LOCK_RETRY = 10
     WRITE_LOCK_SLEEP = 0.01.seconds
     WRITE_LOCK_TTL = 1.minute
 
     enum data_store: {
       redis: 1,
-      db: 2
+      database: 2,
+      fog: 3
     }
 
     class << self
-      def redis_data_key(build_id, chunk_index)
-        "gitlab:ci:trace:#{build_id}:chunks:#{chunk_index}"
+      def all_stores
+        @all_stores ||= self.data_stores.keys
       end
 
-      def redis_data_keys
-        redis.pluck(:build_id, :chunk_index).map do |data|
-          redis_data_key(data.first, data.second)
-        end
+      def persist_store
+        # get first available store from the back of the list
+        all_stores.reverse.find { |store| get_store_class(store).available? }
       end
 
-      def redis_delete_data(keys)
-        return if keys.empty?
-
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.del(keys)
-        end
+      def get_store_class(store)
+        @stores ||= {}
+        @stores[store] ||= "Ci::BuildTraceChunks::#{store.capitalize}".constantize.new
       end
 
       ##
       # FastDestroyAll concerns
       def begin_fast_destroy
-        redis_data_keys
+        all_stores.each_with_object({}) do |result, store|
+          relation = public_send(store)
+          keys = get_store_class(store).keys(relation)
+
+          result[store] = keys if keys.present?
+        end
       end
 
       ##
       # FastDestroyAll concerns
       def finalize_fast_destroy(keys)
-        redis_delete_data(keys)
+        keys.each do |store, value|
+          get_store_class(store).delete_keys(value)
+        end
       end
     end
 
@@ -69,7 +72,7 @@ module Ci
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
 
-      set_data(data.byteslice(0, offset) + new_data)
+      set_data!(data.byteslice(0, offset) + new_data)
     end
 
     def size
@@ -87,51 +90,53 @@ module Ci
     def range
       (start_offset...end_offset)
     end
+    
+    def persisted?
+      !redis?
+    end
 
-    def use_database!
+    def persist!
       in_lock do
-        break if db?
-        break unless size > 0
-
-        self.update!(raw_data: data, data_store: :db)
-        self.class.redis_delete_data([redis_data_key])
+        unsafe_move_to!(self.class.persist_store)
       end
     end
 
     private
 
+    def unsafe_move_to!(new_store)
+      return if data_store == new_store.to_s
+      return unless size > 0
+
+      old_store_class = self.class.get_store_class(data_store)
+
+      self.get_data.tap do |the_data|
+        self.raw_data = nil
+        self.data_store = new_store
+        self.set_data!(the_data)
+      end
+
+      old_store_class.delete_data(self)
+    end
+
     def get_data
-      if redis?
-        redis_data
-      elsif db?
-        raw_data
-      else
-        raise 'Unsupported data store'
-      end&.force_encoding(Encoding::BINARY) # Redis/Database return UTF-8 string as default
+      self.class.get_store_class(data_store).data(self)&.force_encoding(Encoding::BINARY) # Redis/Database return UTF-8 string as default
     end
 
-    def set_data(value)
+    def set_data!(value)
       raise ArgumentError, 'too much data' if value.bytesize > CHUNK_SIZE
 
       in_lock do
-        if redis?
-          redis_set_data(value)
-        elsif db?
-          self.raw_data = value
-        else
-          raise 'Unsupported data store'
-        end
-
+        self.class.get_store_class(data_store).set_data(self, value)
         @data = value
 
         save! if changed?
       end
 
-      schedule_to_db if full?
+      schedule_to_persist if full?
     end
 
-    def schedule_to_db
-      return if db?
+    def schedule_to_persist
+      return if persisted?
 
       Ci::BuildTraceChunkFlushWorker.perform_async(id)
     end
@@ -140,22 +145,6 @@ module Ci
       size == CHUNK_SIZE
     end
 
-    def redis_data
-      Gitlab::Redis::SharedState.with do |redis|
-        redis.get(redis_data_key)
-      end
-    end
-
-    def redis_set_data(data)
-      Gitlab::Redis::SharedState.with do |redis|
-        redis.set(redis_data_key, data, ex: CHUNK_REDIS_TTL)
-      end
-    end
-
-    def redis_data_key
-      self.class.redis_data_key(build_id, chunk_index)
-    end
-
     def in_lock
       write_lock_key = "trace_write:#{build_id}:chunks:#{chunk_index}"
 
diff --git a/app/models/ci/build_trace_chunks/database.rb b/app/models/ci/build_trace_chunks/database.rb
new file mode 100644
index 00000000000..3666d77c790
--- /dev/null
+++ b/app/models/ci/build_trace_chunks/database.rb
@@ -0,0 +1,29 @@
+module Ci
+  module BuildTraceChunks
+    class Database
+      def available?
+        true
+      end
+
+      def keys(relation)
+        []
+      end
+
+      def delete_keys(keys)
+        # no-op
+      end
+
+      def data(model)
+        model.raw_data
+      end
+
+      def set_data(model, data)
+        model.raw_data = data
+      end
+
+      def delete_data(model)
+        model.update_columns(raw_data: nil) unless model.raw_data.nil?
+      end
+    end
+  end
+end
diff --git a/app/models/ci/build_trace_chunks/fog.rb b/app/models/ci/build_trace_chunks/fog.rb
new file mode 100644
index 00000000000..18b09347381
--- /dev/null
+++ b/app/models/ci/build_trace_chunks/fog.rb
@@ -0,0 +1,59 @@
+module Ci
+  module BuildTraceChunks
+    class Fog
+      def available?
+        object_store.enabled
+      end
+
+      def data(model)
+        connection.get_object(bucket_name, key(model)).body
+      end
+
+      def set_data(model, data)
+        connection.put_object(bucket_name, key(model), data)
+      end
+
+      def delete_data(model)
+        delete_keys([[model.build_id, model.chunk_index]])
+      end
+
+      def keys(relation)
+        return [] unless available?
+
+        relation.pluck(:build_id, :chunk_index)
+      end
+
+      def delete_keys(keys)
+        keys.each do |key|
+          connection.delete_object(bucket_name, key_raw(*key))
+        end
+      end
+
+      private
+
+      def key(model)
+        key_raw(model.build_id, model.chunk_index)
+      end
+
+      def key_raw(build_id, chunk_index)
+        "tmp/chunks/builds/#{build_id.to_i}/chunks/#{chunk_index.to_i}.log"
+      end
+
+      def bucket_name
+        return unless available?
+
+        object_store.remote_directory
+      end
+
+      def connection
+        return unless available?
+
+        @connection ||= ::Fog::Storage.new(object_store.connection.to_hash.deep_symbolize_keys)
+      end
+
+      def object_store
+        Gitlab.config.artifacts.object_store
+      end
+    end
+  end
+end
diff --git a/app/models/ci/build_trace_chunks/redis.rb b/app/models/ci/build_trace_chunks/redis.rb
new file mode 100644
index 00000000000..fdb6065e2a0
--- /dev/null
+++ b/app/models/ci/build_trace_chunks/redis.rb
@@ -0,0 +1,51 @@
+module Ci
+  module BuildTraceChunks
+    class Redis
+      CHUNK_REDIS_TTL = 1.week
+
+      def available?
+        true
+      end
+
+      def data(model)
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.get(key(model))
+        end
+      end
+
+      def set_data(model, data)
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.set(key(model), data, ex: CHUNK_REDIS_TTL)
+        end
+      end
+
+      def delete_data(model)
+        delete_keys([[model.build_id, model.chunk_index]])
+      end
+
+      def keys(relation)
+        relation.pluck(:build_id, :chunk_index)
+      end
+
+      def delete_keys(keys)
+        return if keys.empty?
+
+        keys = keys.map { |key| key_raw(*key) }
+
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.del(keys)
+        end
+      end
+
+      private
+
+      def key(model)
+        key_raw(model.build_id, model.chunk_index)
+      end
+
+      def key_raw(build_id, chunk_index)
+        "gitlab:ci:trace:#{build_id.to_i}:chunks:#{chunk_index.to_i}"
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From 082dee862fde985b5d0fc9892059f3f1f2c798bd Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Fri, 15 Jun 2018 15:48:03 +0900
Subject: Fix dead lock by in_lock conflicts.  Shared out in_lock logic. 
 Changed key_raw path of fog store. and some bug fixes to make it functional.

---
 app/models/ci/build.rb                  |  2 +-
 app/models/ci/build_trace_chunk.rb      | 61 +++++++++++++++------------------
 app/models/ci/build_trace_chunks/fog.rb |  4 +--
 3 files changed, 30 insertions(+), 37 deletions(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 41446946a5e..8c90232405e 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -385,7 +385,7 @@ module Ci
     end
 
     def erase_old_trace!
-      update_column(:trace, nil)
+      update_column(:trace, nil) if old_trace
     end
 
     def needs_touch?
diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 6b89efe8eb0..179c5830678 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -1,6 +1,7 @@
 module Ci
   class BuildTraceChunk < ActiveRecord::Base
     include FastDestroyAll
+    include ExclusiveLeaseLock
     extend Gitlab::Ci::Model
 
     belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id
@@ -14,6 +15,8 @@ module Ci
     WRITE_LOCK_SLEEP = 0.01.seconds
     WRITE_LOCK_TTL = 1.minute
 
+    # Note: The ordering of this enum is related to the precedence of persist store.
+    # The bottom item takes the higest precedence, and the top item takes the lowest precedence.
     enum data_store: {
       redis: 1,
       database: 2,
@@ -38,8 +41,8 @@ module Ci
       ##
       # FastDestroyAll concerns
       def begin_fast_destroy
-        all_stores.each_with_object({}) do |result, store|
-          relation = public_send(store)
+        all_stores.each_with_object({}) do |store, result|
+          relation = public_send(store) # rubocop:disable GitlabSecurity/PublicSend
           keys = get_store_class(store).keys(relation)
 
           result[store] = keys if keys.present?
@@ -72,7 +75,13 @@ module Ci
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
 
-      set_data!(data.byteslice(0, offset) + new_data)
+      in_lock(*lock_params) do
+        self.reload if self.persisted?
+
+        unsafe_set_data!(data.byteslice(0, offset) + new_data)
+      end
+
+      schedule_to_persist if full?
     end
 
     def size
@@ -90,13 +99,15 @@ module Ci
     def range
       (start_offset...end_offset)
     end
-    
+
     def persisted?
       !redis?
     end
 
     def persist!
-      in_lock do
+      in_lock(*lock_params) do
+        self.reload if self.persisted?
+
         unsafe_move_to!(self.class.persist_store)
       end
     end
@@ -109,10 +120,10 @@ module Ci
 
       old_store_class = self.class.get_store_class(data_store)
 
-      self.get_data.tap do |the_data|
+      get_data.tap do |the_data|
         self.raw_data = nil
         self.data_store = new_store
-        self.set_data!(the_data)
+        unsafe_set_data!(the_data)
       end
 
       old_store_class.delete_data(self)
@@ -122,17 +133,13 @@ module Ci
       self.class.get_store_class(data_store).data(self)&.force_encoding(Encoding::BINARY) # Redis/Database return UTF-8 string as default
     end
 
-    def set_data!(value)
+    def unsafe_set_data!(value)
       raise ArgumentError, 'too much data' if value.bytesize > CHUNK_SIZE
 
-      in_lock do
-        self.class.get_store_class(data_store).set_data(self, value)
-        @data = value
-
-        save! if changed?
-      end
+      self.class.get_store_class(data_store).set_data(self, value)
+      @data = value
 
-      schedule_to_persist if full?
+      save! if changed?
     end
 
     def schedule_to_persist
@@ -145,25 +152,11 @@ module Ci
       size == CHUNK_SIZE
     end
 
-    def in_lock
-      write_lock_key = "trace_write:#{build_id}:chunks:#{chunk_index}"
-
-      lease = Gitlab::ExclusiveLease.new(write_lock_key, timeout: WRITE_LOCK_TTL)
-      retry_count = 0
-
-      until uuid = lease.try_obtain
-        # Keep trying until we obtain the lease. To prevent hammering Redis too
-        # much we'll wait for a bit between retries.
-        sleep(WRITE_LOCK_SLEEP)
-        break if WRITE_LOCK_RETRY < (retry_count += 1)
-      end
-
-      raise WriteError, 'Failed to obtain write lock' unless uuid
-
-      self.reload if self.persisted?
-      return yield
-    ensure
-      Gitlab::ExclusiveLease.cancel(write_lock_key, uuid)
+    def lock_params
+      ["trace_write:#{build_id}:chunks:#{chunk_index}",
+       { ttl: WRITE_LOCK_TTL,
+         retry_max: WRITE_LOCK_RETRY,
+         sleep_sec: WRITE_LOCK_SLEEP }]
     end
   end
 end
diff --git a/app/models/ci/build_trace_chunks/fog.rb b/app/models/ci/build_trace_chunks/fog.rb
index 18b09347381..7506c40a39d 100644
--- a/app/models/ci/build_trace_chunks/fog.rb
+++ b/app/models/ci/build_trace_chunks/fog.rb
@@ -6,7 +6,7 @@ module Ci
       end
 
       def data(model)
-        connection.get_object(bucket_name, key(model)).body
+        connection.get_object(bucket_name, key(model))[:body]
       end
 
       def set_data(model, data)
@@ -36,7 +36,7 @@ module Ci
       end
 
       def key_raw(build_id, chunk_index)
-        "tmp/chunks/builds/#{build_id.to_i}/chunks/#{chunk_index.to_i}.log"
+        "tmp/builds/#{build_id.to_i}/chunks/#{chunk_index.to_i}.log"
       end
 
       def bucket_name
-- 
cgit v1.2.1


From 494673793d6b4491b2a7843f0614e5bcb3d88a3c Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Mon, 18 Jun 2018 17:56:16 +0900
Subject: Rename persisted? to data_persisted?

---
 app/models/ci/build_trace_chunk.rb | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 179c5830678..59096f54f0b 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -75,9 +75,7 @@ module Ci
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
 
-      in_lock(*lock_params) do
-        self.reload if self.persisted?
-
+      in_lock(*lock_params) do # Write opetation is atomic
         unsafe_set_data!(data.byteslice(0, offset) + new_data)
       end
 
@@ -100,21 +98,19 @@ module Ci
       (start_offset...end_offset)
     end
 
-    def persisted?
+    def data_persisted?
       !redis?
     end
 
-    def persist!
-      in_lock(*lock_params) do
-        self.reload if self.persisted?
-
-        unsafe_move_to!(self.class.persist_store)
+    def persist_data!
+      in_lock(*lock_params) do # Write opetation is atomic
+        unsafe_migrate_to!(self.class.persist_store)
       end
     end
 
     private
 
-    def unsafe_move_to!(new_store)
+    def unsafe_migrate_to!(new_store)
       return if data_store == new_store.to_s
       return unless size > 0
 
@@ -143,7 +139,7 @@ module Ci
     end
 
     def schedule_to_persist
-      return if persisted?
+      return if data_persisted?
 
       Ci::BuildTraceChunkFlushWorker.perform_async(id)
     end
-- 
cgit v1.2.1


From 24ba0989878a363c37d86758844a17a99fc7ae0c Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Mon, 25 Jun 2018 16:19:40 +0900
Subject: Added spec for build trace chunk

---
 app/models/ci/build_trace_chunk.rb | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 59096f54f0b..8a34db798db 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -8,8 +8,6 @@ module Ci
 
     default_value_for :data_store, :redis
 
-    WriteError = Class.new(StandardError)
-
     CHUNK_SIZE = 128.kilobytes
     WRITE_LOCK_RETRY = 10
     WRITE_LOCK_SLEEP = 0.01.seconds
@@ -65,6 +63,7 @@ module Ci
     end
 
     def truncate(offset = 0)
+      raise ArgumentError, 'Fog store does not support truncating' if fog? # If data is null, get_data returns Excon::Error::NotFound
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       return if offset == size # Skip the following process as it doesn't affect anything
 
@@ -72,6 +71,8 @@ module Ci
     end
 
     def append(new_data, offset)
+      raise ArgumentError, 'Fog store does not support appending' if fog? # If data is null, get_data returns Excon::Error::NotFound
+      raise ArgumentError, 'New data is nil' unless new_data
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
 
@@ -98,21 +99,17 @@ module Ci
       (start_offset...end_offset)
     end
 
-    def data_persisted?
-      !redis?
-    end
-
     def persist_data!
       in_lock(*lock_params) do # Write opetation is atomic
-        unsafe_migrate_to!(self.class.persist_store)
+        unsafe_persist_to!(self.class.persist_store)
       end
     end
 
     private
 
-    def unsafe_migrate_to!(new_store)
+    def unsafe_persist_to!(new_store)
       return if data_store == new_store.to_s
-      return unless size > 0
+      raise ArgumentError, 'Can not persist empty data' unless size > 0
 
       old_store_class = self.class.get_store_class(data_store)
 
@@ -130,7 +127,7 @@ module Ci
     end
 
     def unsafe_set_data!(value)
-      raise ArgumentError, 'too much data' if value.bytesize > CHUNK_SIZE
+      raise ArgumentError, 'New data size exceeds chunk size' if value.bytesize > CHUNK_SIZE
 
       self.class.get_store_class(data_store).set_data(self, value)
       @data = value
@@ -144,6 +141,10 @@ module Ci
       Ci::BuildTraceChunkFlushWorker.perform_async(id)
     end
 
+    def data_persisted?
+      !redis?
+    end
+
     def full?
       size == CHUNK_SIZE
     end
-- 
cgit v1.2.1


From 58a1a0b70c7df0947864d0be933faf0153b537ec Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Mon, 25 Jun 2018 19:59:28 +0900
Subject: Support append/truncate for fog store

---
 app/models/ci/build_trace_chunk.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 8a34db798db..4362570b5ee 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -63,7 +63,6 @@ module Ci
     end
 
     def truncate(offset = 0)
-      raise ArgumentError, 'Fog store does not support truncating' if fog? # If data is null, get_data returns Excon::Error::NotFound
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       return if offset == size # Skip the following process as it doesn't affect anything
 
@@ -71,7 +70,6 @@ module Ci
     end
 
     def append(new_data, offset)
-      raise ArgumentError, 'Fog store does not support appending' if fog? # If data is null, get_data returns Excon::Error::NotFound
       raise ArgumentError, 'New data is nil' unless new_data
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
@@ -124,6 +122,8 @@ module Ci
 
     def get_data
       self.class.get_store_class(data_store).data(self)&.force_encoding(Encoding::BINARY) # Redis/Database return UTF-8 string as default
+    rescue Excon::Error::NotFound
+      # If the data store is :fog and the file does not exist in the object storage, this method returns nil.
     end
 
     def unsafe_set_data!(value)
-- 
cgit v1.2.1


From b223f7b7a081be31cf5cc6026decad13bd79c813 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Tue, 3 Jul 2018 14:33:11 +0900
Subject: Rename persistable_store instead of persist_store

---
 app/models/ci/build_trace_chunk.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 4362570b5ee..e7c56b94751 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -26,7 +26,7 @@ module Ci
         @all_stores ||= self.data_stores.keys
       end
 
-      def persist_store
+      def persistable_store
         # get first available store from the back of the list
         all_stores.reverse.find { |store| get_store_class(store).available? }
       end
@@ -99,7 +99,7 @@ module Ci
 
     def persist_data!
       in_lock(*lock_params) do # Write opetation is atomic
-        unsafe_persist_to!(self.class.persist_store)
+        unsafe_persist_to!(self.class.persistable_store)
       end
     end
 
-- 
cgit v1.2.1


From 902e69dedd1b4c60ce109c346b65c5e61a46ff4a Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Tue, 3 Jul 2018 14:48:00 +0900
Subject: Fix error message

---
 app/models/ci/build_trace_chunk.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index e7c56b94751..b724d0cb517 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -70,7 +70,7 @@ module Ci
     end
 
     def append(new_data, offset)
-      raise ArgumentError, 'New data is nil' unless new_data
+      raise ArgumentError, 'New data is missing' unless new_data
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize)
 
-- 
cgit v1.2.1


From 93a964d449ad29e94e785209d7ecde217a8e9b25 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Tue, 3 Jul 2018 16:20:27 +0900
Subject: Add spec for ExclusiveLeaseHelpers

---
 app/models/ci/build_trace_chunk.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index b724d0cb517..43bede7638c 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -1,7 +1,7 @@
 module Ci
   class BuildTraceChunk < ActiveRecord::Base
     include FastDestroyAll
-    include ExclusiveLeaseLock
+    include ::Gitlab::ExclusiveLeaseHelpers
     extend Gitlab::Ci::Model
 
     belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id
-- 
cgit v1.2.1


From 7b06877330201deeef91fc4d1d1fedeb599c26ec Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Wed, 4 Jul 2018 13:29:47 +0900
Subject: Rename retry_max left overs to retries

---
 app/models/ci/build_trace_chunk.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb
index 43bede7638c..b442de34061 100644
--- a/app/models/ci/build_trace_chunk.rb
+++ b/app/models/ci/build_trace_chunk.rb
@@ -152,7 +152,7 @@ module Ci
     def lock_params
       ["trace_write:#{build_id}:chunks:#{chunk_index}",
        { ttl: WRITE_LOCK_TTL,
-         retry_max: WRITE_LOCK_RETRY,
+         retries: WRITE_LOCK_RETRY,
          sleep_sec: WRITE_LOCK_SLEEP }]
     end
   end
-- 
cgit v1.2.1


From 1f1ef460ca3a02597b4e37af67251f3824cf4ea8 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Wed, 4 Jul 2018 13:30:45 +0900
Subject: Decouple optimization for erasing trace

---
 app/models/ci/build.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/models/ci')

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 8c90232405e..41446946a5e 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -385,7 +385,7 @@ module Ci
     end
 
     def erase_old_trace!
-      update_column(:trace, nil) if old_trace
+      update_column(:trace, nil)
     end
 
     def needs_touch?
-- 
cgit v1.2.1