diff options
-rw-r--r-- | app/models/ci/build.rb | 37 | ||||
-rw-r--r-- | changelogs/unreleased/expired-ci-artifacts.yml | 5 | ||||
-rw-r--r-- | db/migrate/20180119160751_optimize_ci_job_artifacts.rb | 23 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | spec/factories/ci/builds.rb | 4 |
5 files changed, 65 insertions, 6 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 20534b8eed0..7312e704932 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -41,12 +41,41 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } + + # This convoluted mess is because we need to handle two cases of + # artifact files during the migration. And a simple OR clause + # makes it impossible to optimize. + + # Instead we want to use UNION ALL and do two carefully + # constructed disjoint queries. But Rails cannot handle UNION or + # UNION ALL queries so we do the query in a subquery and wrap it + # in an otherwise redundant WHERE IN query (IN is fine for + # non-null columns). + + # This should all be ripped out when the migration is finished and + # replaced with just the new storage to avoid the extra work. + scope :with_artifacts, ->() do - where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', - '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id')) + old = Ci::Build.select(:id).where(%q[artifacts_file <> '']) + new = Ci::Build.select(:id).where(%q[(artifacts_file IS NULL OR artifacts_file = '') AND EXISTS (?)], + Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id')) + where('ci_builds.id IN (? UNION ALL ?)', old, new) end - scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) } - scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } + + scope :with_artifacts_not_expired, ->() do + old = Ci::Build.select(:id).where(%q[artifacts_file <> '' AND (artifacts_expire_at IS NULL OR artifacts_expire_at > ?)], Time.now) + new = Ci::Build.select(:id).where(%q[(artifacts_file IS NULL OR artifacts_file = '') AND EXISTS (?)], + Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id AND (expire_at IS NULL OR expire_at > ?)', Time.now)) + where('ci_builds.id IN (? UNION ALL ?)', old, new) + end + + scope :with_expired_artifacts, ->() do + old = Ci::Build.select(:id).where(%q[artifacts_file <> '' AND artifacts_expire_at < ?], Time.now) + new = Ci::Build.select(:id).where(%q[(artifacts_file IS NULL OR artifacts_file = '') AND EXISTS (?)], + Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id AND expire_at < ?', Time.now)) + where('ci_builds.id IN (? UNION ALL ?)', old, new) + end + scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) } scope :ref_protected, -> { where(protected: true) } diff --git a/changelogs/unreleased/expired-ci-artifacts.yml b/changelogs/unreleased/expired-ci-artifacts.yml new file mode 100644 index 00000000000..2fcbdb02f84 --- /dev/null +++ b/changelogs/unreleased/expired-ci-artifacts.yml @@ -0,0 +1,5 @@ +--- +title: Change SQL for expired artifacts to use new ci_job_artifacts.expire_at +merge_request: 16578 +author: +type: performance diff --git a/db/migrate/20180119160751_optimize_ci_job_artifacts.rb b/db/migrate/20180119160751_optimize_ci_job_artifacts.rb new file mode 100644 index 00000000000..9b4340ed7b7 --- /dev/null +++ b/db/migrate/20180119160751_optimize_ci_job_artifacts.rb @@ -0,0 +1,23 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class OptimizeCiJobArtifacts < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + # job_id is just here to be a covering index for index only scans + # since we'll almost always be joining against ci_builds on job_id + add_concurrent_index(:ci_job_artifacts, [:expire_at, :job_id]) + add_concurrent_index(:ci_builds, [:artifacts_expire_at], where: "artifacts_file <> ''") + end + + def down + remove_concurrent_index(:ci_job_artifacts, [:expire_at, :job_id]) + remove_concurrent_index(:ci_builds, [:artifacts_expire_at], where: "artifacts_file <> ''") + end +end diff --git a/db/schema.rb b/db/schema.rb index d07a4c31618..57b3f4c9988 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -293,6 +293,7 @@ ActiveRecord::Schema.define(version: 20180206200543) do t.integer "failure_reason" end + add_index "ci_builds", ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree add_index "ci_builds", ["commit_id", "status", "type"], name: "index_ci_builds_on_commit_id_and_status_and_type", using: :btree @@ -333,6 +334,7 @@ ActiveRecord::Schema.define(version: 20180206200543) do t.string "file" end + add_index "ci_job_artifacts", ["expire_at", "job_id"], name: "index_ci_job_artifacts_on_expire_at_and_job_id", using: :btree add_index "ci_job_artifacts", ["job_id", "file_type"], name: "index_ci_job_artifacts_on_job_id_and_file_type", unique: true, using: :btree add_index "ci_job_artifacts", ["project_id"], name: "index_ci_job_artifacts_on_project_id", using: :btree diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 6ba599cdf83..f6ba3a581ca 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -180,8 +180,8 @@ FactoryBot.define do trait :artifacts do after(:create) do |build| - create(:ci_job_artifact, :archive, job: build) - create(:ci_job_artifact, :metadata, job: build) + create(:ci_job_artifact, :archive, job: build, expire_at: build.artifacts_expire_at) + create(:ci_job_artifact, :metadata, job: build, expire_at: build.artifacts_expire_at) build.reload end end |