Commit eb12e3ab authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'mr-lock-problem' into 'master'

Fix merge request lock problem

Merge Request lock problem
MR can be stuck in lock state if satelitte timeout >= unicorn timeout.

Issue explanation:

* Person press Accept button
* unicorn worker starts, lock MR and trigger satellite
* satellite does merge but very long (30 seconds)
* unicorn timeout reaches earlier then satellite timeout
* unicorn kills worker because of timeout leaving MR in lock state.

Fix:

* set locked_at date
* if MR was locked for too long - close it automatically

- - -

Fixes #1674

See merge request !1306
parents 805e61ce 64874193
...@@ -225,6 +225,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -225,6 +225,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@allowed_to_merge = allowed_to_merge? @allowed_to_merge = allowed_to_merge?
@show_merge_controls = @merge_request.open? && @commits.any? && @allowed_to_merge @show_merge_controls = @merge_request.open? && @commits.any? && @allowed_to_merge
@source_branch = @merge_request.source_project.repository.find_branch(@merge_request.source_branch).try(:name) @source_branch = @merge_request.source_project.repository.find_branch(@merge_request.source_branch).try(:name)
if @merge_request.locked_long_ago?
@merge_request.unlock_mr
@merge_request.close
end
end end
def allowed_to_merge? def allowed_to_merge?
......
...@@ -70,6 +70,16 @@ class MergeRequest < ActiveRecord::Base ...@@ -70,6 +70,16 @@ class MergeRequest < ActiveRecord::Base
transition locked: :reopened transition locked: :reopened
end end
after_transition any => :locked do |merge_request, transition|
merge_request.locked_at = Time.now
merge_request.save
end
after_transition :locked => (any - :locked) do |merge_request, transition|
merge_request.locked_at = nil
merge_request.save
end
state :opened state :opened
state :reopened state :reopened
state :closed state :closed
...@@ -336,4 +346,8 @@ class MergeRequest < ActiveRecord::Base ...@@ -336,4 +346,8 @@ class MergeRequest < ActiveRecord::Base
source_project.repository.branch_names source_project.repository.branch_names
end end
end end
def locked_long_ago?
locked_at && locked_at < (Time.now - 1.day)
end
end end
...@@ -11,7 +11,9 @@ ...@@ -11,7 +11,9 @@
- if @merge_request.closed? - if @merge_request.closed?
%h4 %h4
Closed by #{link_to_member(@project, @merge_request.closed_event.author, avatar: false)} Closed
- if @merge_request.closed_event
by #{link_to_member(@project, @merge_request.closed_event.author, avatar: false)}
#{time_ago_with_tooltip(@merge_request.closed_event.created_at)} #{time_ago_with_tooltip(@merge_request.closed_event.created_at)}
%p Changes were not merged into target branch %p Changes were not merged into target branch
......
class AddLockedAtToMergeRequest < ActiveRecord::Migration
def change
add_column :merge_requests, :locked_at, :datetime
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20141121161704) do ActiveRecord::Schema.define(version: 20141205134006) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -181,6 +181,7 @@ ActiveRecord::Schema.define(version: 20141121161704) do ...@@ -181,6 +181,7 @@ ActiveRecord::Schema.define(version: 20141121161704) do
t.integer "iid" t.integer "iid"
t.text "description" t.text "description"
t.integer "position", default: 0 t.integer "position", default: 0
t.datetime "locked_at"
end end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment