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, 6 insertions, 65 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 490edf4ac57..ee987949080 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -41,41 +41,12 @@ 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 - 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) + where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', + '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id')) end - - 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 :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 :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 deleted file mode 100644 index 2fcbdb02f84..00000000000 --- a/changelogs/unreleased/expired-ci-artifacts.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -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 deleted file mode 100644 index 9b4340ed7b7..00000000000 --- a/db/migrate/20180119160751_optimize_ci_job_artifacts.rb +++ /dev/null @@ -1,23 +0,0 @@ -# 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 6b43fc8403c..b281be110da 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -293,7 +293,6 @@ ActiveRecord::Schema.define(version: 20180208183958) 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 @@ -334,7 +333,6 @@ ActiveRecord::Schema.define(version: 20180208183958) 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 f6ba3a581ca..6ba599cdf83 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, expire_at: build.artifacts_expire_at) - create(:ci_job_artifact, :metadata, job: build, expire_at: build.artifacts_expire_at) + create(:ci_job_artifact, :archive, job: build) + create(:ci_job_artifact, :metadata, job: build) build.reload end end |