Move job timeout information to new ci_builds_metadata table

app/assets/javascripts/jobs/components/sidebar_details_block.vue

@@ -45,10 +45,10 @@
         return `#${this.job.runner.id}`;
       },
       timeout() {
-        let t = `${this.job.timeout.value}`;
+        let t = `${this.job.metadata.used_timeout_human_readable}`;
 
-        if (this.job.timeout.source != null) {
-          t += ` (from ${this.job.timeout.source})`;
+        if (this.job.metadata.timeout_source != null) {
+          t += ` (from ${this.job.metadata.timeout_source})`;
         }
 
         return t;
@@ -130,7 +130,7 @@
         />
         <detail-row
           class="js-job-timeout"
-          v-if="job.timeout"
+          v-if="job.metadata.used_timeout_human_readable"
           title="Timeout"
           :help-url="runnerHelpUrl"
           :value="timeout"

app/models/ci/build.rb

@@ -6,7 +6,6 @@ module Ci
     include ObjectStorage::BackgroundMove
     include Presentable
     include Importable
-    include ChronicDurationAttribute
 
     MissingDependenciesError = Class.new(StandardError)
 
@@ -25,6 +24,8 @@ module Ci
     has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
     has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
 
+    has_one :metadata, class_name: 'Ci::BuildMetadata'
+
     # The "environment" field for builds is a String, and is the unexpanded name
     def persisted_environment
       @persisted_environment ||= Environment.find_by(
@@ -84,6 +85,10 @@ module Ci
     before_save :ensure_token
     before_destroy { unscoped_project }
 
+    before_create do |build|
+      build.metadata = Ci::BuildMetadata.new
+    end
+
     after_create unless: :importing? do |build|
       run_after_commit { BuildHooksWorker.perform_async(build.id) }
     end
@@ -91,14 +96,6 @@ module Ci
     after_commit :update_project_statistics_after_save, on: [:create, :update]
     after_commit :update_project_statistics, on: :destroy
 
-    chronic_duration_attr_reader :used_timeout_human_readable, :used_timeout
-
-    enum timeout_source: {
-        unknown_timeout_source: nil,
-        project_timeout_source: 1,
-        runner_timeout_source: 2
-    }
-
     class << self
       # This is needed for url_for to work,
       # as the controller is JobsController
@@ -164,8 +161,7 @@ module Ci
       end
 
       before_transition pending: :running do |build|
-        build.used_timeout = build.timeout
-        build.timeout_source = build.timeout < build.project.build_timeout ? :runner_timeout_source : :project_timeout_source
+        build.metadata.save_timeout_state!
       end
     end
 

app/models/ci/build_metadata.rb

+module Ci
+  class BuildMetadata < ActiveRecord::Base
+    extend Gitlab::Ci::Model
+    include Presentable
+    include ChronicDurationAttribute
+
+    self.table_name = 'ci_builds_metadata'
+
+    belongs_to :build, class_name: 'Ci::Build'
+
+    chronic_duration_attr_reader :used_timeout_human_readable, :used_timeout
+
+    enum timeout_source: {
+        unknown_timeout_source: nil,
+        project_timeout_source: 1,
+        runner_timeout_source: 2
+    }
+
+    def save_timeout_state!
+      self.used_timeout = build.timeout
+      self.timeout_source = build.timeout < build.project.build_timeout ? :runner_timeout_source : :project_timeout_source
+      save!
+    end
+  end
+end

app/presenters/ci/build_metadata_presenter.rb

+module Ci
+  class BuildMetadataPresenter < Gitlab::View::Presenter::Delegated
+
+    TIMEOUT_SOURCES = {
+        unknown_timeout_source: nil,
+        project_timeout_source: 'project',
+        runner_timeout_source: 'runner'
+    }.freeze
+
+    presents :metadata
+
+    def timeout_source
+      return unless metadata.timeout_source?
+
+      TIMEOUT_SOURCES[metadata.timeout_source.to_sym] ||
+          metadata.timeout_source
+    end
+
+  end
+end

app/presenters/ci/build_presenter.rb

 module Ci
   class BuildPresenter < Gitlab::View::Presenter::Delegated
 
-    TIMEOUT_SOURCES = {
-        unknown_timeout_source: nil,
-        project_timeout_source: 'project',
-        runner_timeout_source: 'runner'
-    }.freeze
-
     presents :build
 
     def erased_by_user?
@@ -25,13 +19,6 @@ module Ci
       end
     end
 
-    def timeout_source
-      return unless build.timeout_source?
-
-      TIMEOUT_SOURCES[build.timeout_source.to_sym] ||
-          build.timeout_source
-    end
-
     def trigger_variables
       return [] unless trigger_request
 

app/serializers/build_details_entity.rb

@@ -5,10 +5,7 @@ class BuildDetailsEntity < JobEntity
   expose :runner, using: RunnerEntity
   expose :pipeline, using: PipelineEntity
 
-  expose :timeout, if: -> (*) { !build.used_timeout.nil? } do |build|
-    { value: build.used_timeout_human_readable,
-      source: build.present.timeout_source }
-  end
+  expose :metadata, using: BuildMetadataEntity
 
   expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity
   expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :erase_build, build) } do |build|

app/serializers/build_metadata_entity.rb

+class BuildMetadataEntity < Grape::Entity
+
+  expose :used_timeout_human_readable do |metadata|
+    metadata.used_timeout_human_readable unless metadata.used_timeout.nil?
+  end
+
+  expose :timeout_source do |metadata|
+    metadata.present.timeout_source
+  end
+end

db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb

-class AddUsedTimeoutAndTimeoutSourceColumnsToCiBuilds < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  def change
-    add_column :ci_builds, :used_timeout, :integer
-    add_column :ci_builds, :timeout_source, :integer
-  end
-end

db/migrate/20180301010859_create_ci_builds_metadata_table.rb

+class CreateCiBuildsMetadataTable < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :ci_builds_metadata do |t|
+      t.integer :build_id, null: false
+      t.integer :used_timeout
+      t.integer :timeout_source
+    end
+  end
+end

db/migrate/20180301011323_add_build_foreign_key_to_ci_builds_metadata.rb

+class AddBuildForeignKeyToCiBuildsMetadata < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key(:ci_builds_metadata, :ci_builds, column: :build_id)
+  end
+
+  def down
+    remove_foreign_key(:ci_builds_metadata, column: :build_id)
+  end
+end

db/schema.rb

@@ -311,8 +311,6 @@ ActiveRecord::Schema.define(version: 20180327101207) do
     t.integer "artifacts_metadata_store"
     t.boolean "protected"
     t.integer "failure_reason"
-    t.integer "used_timeout"
-    t.integer "timeout_source"
   end
 
   add_index "ci_builds", ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree
@@ -331,6 +329,12 @@ ActiveRecord::Schema.define(version: 20180327101207) do
   add_index "ci_builds", ["updated_at"], name: "index_ci_builds_on_updated_at", using: :btree
   add_index "ci_builds", ["user_id"], name: "index_ci_builds_on_user_id", using: :btree
 
+  create_table "ci_builds_metadata", force: :cascade do |t|
+    t.integer "build_id", null: false
+    t.integer "used_timeout"
+    t.integer "timeout_source"
+  end
+
   create_table "ci_group_variables", force: :cascade do |t|
     t.string "key", null: false
     t.text "value"
@@ -460,8 +464,8 @@ ActiveRecord::Schema.define(version: 20180327101207) do
     t.boolean "run_untagged", default: true, null: false
     t.boolean "locked", default: false, null: false
     t.integer "access_level", default: 0, null: false
-    t.string "ip_address"
     t.integer "maximum_job_timeout"
+    t.string "ip_address"
   end
 
   add_index "ci_runners", ["contacted_at"], name: "index_ci_runners_on_contacted_at", using: :btree
@@ -2031,6 +2035,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do
   add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify
   add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade
   add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade
+  add_foreign_key "ci_builds_metadata", "ci_builds", column: "build_id", name: "fk_e20479742e", on_delete: :cascade
   add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade
   add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade
   add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade

spec/javascripts/jobs/mock_data.js

@@ -115,9 +115,9 @@ export default {
       commit_path: '/root/ci-mock/commit/c58647773a6b5faf066d4ad6ff2c9fbba5f180f6',
     },
   },
-  timeout: {
-    value: '1m 40s',
-    source: 'runner',
+  metadata: {
+    used_timeout_human_readable: '1m 40s',
+    timeout_source: 'runner',
   },
   merge_request: {
     iid: 2,

spec/models/ci/build_spec.rb

@@ -2047,11 +2047,11 @@ describe Ci::Build do
 
     shared_examples 'saves data on transition' do
       it 'saves used_timeout' do
-        expect { job.run! }.to change { job.reload.used_timeout }.from(nil).to(expected_timeout)
+        expect { job.run! }.to change { job.reload.metadata.used_timeout }.from(nil).to(expected_timeout)
       end
 
       it 'saves timeout_source' do
-        expect { job.run! }.to change { job.reload.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source)
+        expect { job.run! }.to change { job.reload.metadata.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source)
       end
     end
 

spec/models/concerns/chronic_duration_attribute_spec.rb

@@ -48,7 +48,7 @@ end
 describe 'ChronicDurationAttribute - reader' do
   let(:source_field) {:used_timeout}
   let(:virtual_field) {:used_timeout_human_readable}
-  subject {Ci::Build.new}
+  subject {Ci::BuildMetadata.new}
 
   it "doesn't contain dynamically created writer method" do
     expect(subject.class).not_to be_public_method_defined("#{virtual_field}=")

spec/services/ci/retry_build_service_spec.rb

@@ -30,7 +30,7 @@ describe Ci::RetryBuildService do
        runner_id tag_taggings taggings tags trigger_request_id
        user_id auto_canceled_by_id retried failure_reason
        artifacts_file_store artifacts_metadata_store
-       used_timeout timeout_source].freeze
+       metadata].freeze
 
   shared_examples 'build duplication' do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }