diff options
author | Kamil Trzcinski <ayufan@ayufan.eu> | 2016-10-20 10:33:44 +0300 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2016-10-26 12:37:23 +0300 |
commit | 5d7ee7a1b6c818dd0ccba6a393875072dabd7eba (patch) | |
tree | 80067ea849e46a106e3177de7bcba050748c164b | |
parent | 198ae21e2ccd88f2670faf90d69799f2b3294b73 (diff) |
Use optimistic locking
-rw-r--r-- | app/models/ci/pipeline.rb | 12 | ||||
-rw-r--r-- | app/services/ci/process_pipeline_service.rb | 23 | ||||
-rw-r--r-- | app/services/ci/register_build_service.rb | 11 | ||||
-rw-r--r-- | db/migrate/20161021114307_add_lock_version_to_build_and_pipelines.rb | 14 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/optimistic_locking.rb | 13 |
6 files changed, 50 insertions, 25 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index adda3b8f40c..27178e2a6c1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -30,23 +30,23 @@ module Ci end event :run do - transition any => :running + transition any - [:running] => :running end event :skip do - transition any => :skipped + transition any - [:skipped] => :skipped end event :drop do - transition any => :failed + transition any - [:failed] => :failed end event :succeed do - transition any => :success + transition any - [:success] => :success end event :cancel do - transition any => :canceled + transition any - [:canceled] => :canceled end # IMPORTANT @@ -260,7 +260,7 @@ module Ci end def update_status - with_lock do + Gitlab::OptimisticLocking.retry_lock(build) do case latest_builds_status when 'pending' then enqueue when 'running' then run diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index d3dd30b2588..3b010269e0c 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -10,17 +10,14 @@ module Ci create_builds! end - @pipeline.with_lock do - new_builds = - stage_indexes_of_created_builds.map do |index| - process_stage(index) - end + new_builds = + stage_indexes_of_created_builds.map do |index| + process_stage(index) + end - @pipeline.update_status + @pipeline.update_status - # Return a flag if a when builds got enqueued - new_builds.flatten.any? - end + new_builds.flatten.any? end private @@ -32,9 +29,11 @@ module Ci def process_stage(index) current_status = status_for_prior_stages(index) - created_builds_in_stage(index).select do |build| - if HasStatus::COMPLETED_STATUSES.include?(current_status) - process_build(build, current_status) + if HasStatus::COMPLETED_STATUSES.include?(current_status) + created_builds_in_stage(index).select do |build| + Gitlab::OptimisticLocking.retry_lock(build) do |build| + process_build(build, current_status) + end end end end diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb index 6973191b203..8d3bc8e2dee 100644 --- a/app/services/ci/register_build_service.rb +++ b/app/services/ci/register_build_service.rb @@ -28,17 +28,14 @@ module Ci if build # In case when 2 runners try to assign the same build, second runner will be declined - # with StateMachines::InvalidTransition in run! method. - build.with_lock do - build.runner_id = current_runner.id - build.save! - build.run! - end + # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. + build.runner_id = current_runner.id + build.run! end build - rescue StateMachines::InvalidTransition + rescue StateMachines::InvalidTransition, StaleObjectError nil end 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 02282b0f666..08aacc3eb37 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -189,6 +189,7 @@ ActiveRecord::Schema.define(version: 20161024042317) 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: 20161024042317) 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 diff --git a/lib/gitlab/optimistic_locking.rb b/lib/gitlab/optimistic_locking.rb new file mode 100644 index 00000000000..7668c518bc5 --- /dev/null +++ b/lib/gitlab/optimistic_locking.rb @@ -0,0 +1,13 @@ +module Gitlab + module OptimisticLocking + def retry_lock(subject, &block) + while true do + begin + return yield subject + rescue StaleObjectError + subject.reload + end + end + end + end +end |