summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2018-02-16 22:19:22 +0000
committerStan Hu <stanhu@gmail.com>2018-02-16 22:19:22 +0000
commita978a5e01de26ad8d96312cb0a07087ec2dccb21 (patch)
tree4820e6b5a52fc0c4b98b3d7d4b9463b3cbfd52c2
parentb236348388c46c0550ec6844df35ec2689c4060b (diff)
downloadgitlab-ce-a978a5e01de26ad8d96312cb0a07087ec2dccb21.tar.gz
Revert "Merge branch 'expired-ci-artifacts' into 'master'"
This reverts merge request !16578
-rw-r--r--app/models/ci/build.rb37
-rw-r--r--changelogs/unreleased/expired-ci-artifacts.yml5
-rw-r--r--db/migrate/20180119160751_optimize_ci_job_artifacts.rb23
-rw-r--r--db/schema.rb2
-rw-r--r--spec/factories/ci/builds.rb4
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