summaryrefslogtreecommitdiff
path: root/db
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2016-10-28 14:41:24 +0000
committerStan Hu <stanhu@gmail.com>2016-10-28 14:41:24 +0000
commitd306b0d7c2c1f9384382c2a90a9d7c43bd20573c (patch)
tree307b59b8e83d752eba286ab883a4a2b723deee3e /db
parent144358e98ee1b25b61854a3471e21e100ace9db5 (diff)
parent2822526e7b996c90fb3bbd7c286c2777e5e37360 (diff)
downloadgitlab-ce-d306b0d7c2c1f9384382c2a90a9d7c43bd20573c.tar.gz
Merge branch 'use-optimistic-locking' into 'master'
Use optimistic locking ## What does this MR do? Removes the usage of pessimistic locking in favor of optimistic which is way cheaper and doesn't block database operation. Since this is very simple change it should be safe. If we receive `StaleObjectError` message we will reload object a retry operations in lock. However, I still believe that we need this one: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7005 as this will reduce a load on Database and FS. This changes a behavior from: ### Pesimistic locking (previous behavior) #### For updating 1. SELECT * FOR UPDATE (other updates wait on this) 2. we update ci_pipeline 3. latest_build_status 4. enqueue: (use: transition :created -> :pending) 5. [state_machine] we are in state created, we can go to pending 6. [state_machine] ci_pipeline.status = created 7. [state_machine] ci_pipeline.save 8. [state_machine] after_transition: (if for success): PipelineSuccessWorker on Sidekiq 9. release DB lock #### If no update is required 1. SELECT * FOR UPDATE (other updates wait on this) 2. we update ci_pipeline 3. latest_build_status 4. we are in pending, we can't transition to pending, because it's forbidden 5. release DB lock ### Optimistic locking (implemented by this MR) #### For updating 1. latest_build_status 2. enqueue: (use `transition :created -> :pending`) 3. [state_machine] we are in state created, we can go to pending 4. [state_machine] ci_pipeline.status = created 5. [state_machine] ci_pipeline.save 6. [state_machine] [save] where(lock_version: ci_pipeline.lock_version).update_all(status: :created, updated_at: Time.now) 7. [state_machine] [save] unless we_updated_row then raise ObjectInconsistentError #### If no update is required 1. we update ci_pipeline 2. latest_build_status 3. we are in pending, we can't transition to pending, because it's forbidden ## Why was this MR needed? We have been seeing a number of problems when we migrated Pipeline/Build processing to Sidekiq. Especially we started seeing a lot of blocking queries. We used a pessimistic locking which doesn't seem to be required. This effectively allows us to fix our issues with blocked queries by using more efficient method of operation. ## What are the relevant issue numbers? Issues: https://gitlab.com/gitlab-com/infrastructure/issues/623 and https://gitlab.com/gitlab-com/infrastructure/issues/584, but also there's a bunch of Merge Requests that try to improve behavior of scheduled jobs. cc @pcarranza @yorickpeterse @stanhu See merge request !7040
Diffstat (limited to 'db')
-rw-r--r--db/migrate/20161021114307_add_lock_version_to_build_and_pipelines.rb14
-rw-r--r--db/schema.rb2
2 files changed, 16 insertions, 0 deletions
diff --git a/db/migrate/20161021114307_add_lock_version_to_build_and_pipelines.rb b/db/migrate/20161021114307_add_lock_version_to_build_and_pipelines.rb
new file mode 100644
index 00000000000..b47f3aa2810
--- /dev/null
+++ b/db/migrate/20161021114307_add_lock_version_to_build_and_pipelines.rb
@@ -0,0 +1,14 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLockVersionToBuildAndPipelines < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ def change
+ add_column :ci_builds, :lock_version, :integer
+ add_column :ci_commits, :lock_version, :integer
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5bbfba0d7dd..54b5fc83be0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -189,6 +189,7 @@ ActiveRecord::Schema.define(version: 20161025231710) do
t.text "yaml_variables"
t.datetime "queued_at"
t.string "token"
+ t.integer "lock_version"
end
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
@@ -219,6 +220,7 @@ ActiveRecord::Schema.define(version: 20161025231710) do
t.datetime "finished_at"
t.integer "duration"
t.integer "user_id"
+ t.integer "lock_version"
end
add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree