touch module outside content_tag save txn to prevent deadlock

fixes CNVS-7757

test plan:
  - open two Rails consoles
  - in each, create a bunch of items in the same
    module at the same time, e.g.

    mod = ContextModule.last
    100.times do |x|
      mod.add_item type: 'context_module_sub_header',
                   title: "item #{x}"
    end
  - each console should finish adding its items

 or, when deployed in a production-like environment
 (with multiple app servers, or at least a multithreaded server)
  - generate a curl command line to add an item to a module
    (works with a subheader, so you don't need corresponding
    assets)

    curl -H "Authorization: Bearer {token} \
      https://<canvas>/api/v1/courses/XXX/modules/YYY/items \
      -F module_item[title]=newthing -F module_item[type]=SubHeader &

    (the trailing &, after a space, means run in the background;
     that is, don't wait for the preceding request to finish
     before returning to the command line)
  - run the preceding command several times in rapid succession
    (up, enter, up, enter, etc.)
    (I was able to reproduce in less than 10 requests)
  - look at the json blocks returned, and make sure none of them
    are an internal server error (if they are, there will be an
    error report number you can look up and see if the deadlock
    happened)

Change-Id: I63834cfd98393e5207625db27d18451103aa2944
Reviewed-on: https://gerrit.instructure.com/23783
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2013-08-27 10:46:08 -06:00
parent 8554a05f71
commit fc5de0f52d
4 changed files with 27 additions and 5 deletions

View File

@ -325,8 +325,7 @@ class ContextModulesController < ApplicationController
def add_item
@module = @context.context_modules.not_deleted.find(params[:context_module_id])
if authorized_action(@module, @current_user, :update)
@tag = @module.add_item(params[:item]) #@item)
@module.touch
@tag = @module.add_item(params[:item])
render :json => @tag.to_json
end
end
@ -336,7 +335,6 @@ class ContextModulesController < ApplicationController
if authorized_action(@tag.context_module, @current_user, :update)
@module = @tag.context_module
@tag.destroy
@module.touch
render :json => @tag.to_json
end
end

View File

@ -42,7 +42,7 @@ class ContentTag < ActiveRecord::Base
validates_length_of :comments, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true
before_save :default_values
after_save :update_could_be_locked
after_save :touch_context_module
after_save :touch_context_module_after_transaction
after_save :touch_context_if_learning_outcome
include CustomValidations
validates_as_url :url
@ -71,8 +71,16 @@ class ContentTag < ActiveRecord::Base
attr_accessor :skip_touch
def touch_context_module
ContentTag.touch_context_modules([self.context_module_id]) unless skip_touch.present?
return true if skip_touch.present?
ContentTag.touch_context_modules([self.context_module_id])
end
def touch_context_module_after_transaction
connection.after_transaction_commit {
touch_context_module
}
end
private :touch_context_module_after_transaction
def self.touch_context_modules(ids=[])
ContextModule.where(:id => ids).update_all(:updated_at => Time.now.utc) unless ids.empty?

View File

@ -34,6 +34,8 @@ describe ContextModule do
response.body.should match(/My Sub Header Title/)
content_tag.update_attributes(:title => "My New Title")
run_transaction_commit_callbacks
get "/courses/#{@course.id}/modules"
response.body.should match(/My New Title/)
end

View File

@ -259,6 +259,17 @@ describe ContentTag do
test_url_validation(ContentTag.create!(:content => quiz, :context => @course))
end
it "should touch the module after committing the save" do
course
mod = @course.context_modules.create!
yesterday = 1.day.ago
ContextModule.where(:id => mod).update_all(:updated_at => yesterday)
tag = mod.add_item :type => 'context_module_sub_header', :title => 'blah'
mod.reload.updated_at.to_i.should == yesterday.to_i
run_transaction_commit_callbacks
mod.reload.updated_at.should > 5.seconds.ago
end
it "should allow skipping touches on save" do
course
@assignment = @course.assignments.create!(:title => "some assignment")
@ -268,6 +279,7 @@ describe ContentTag do
:title => 'some assignment (renamed)',
:id => @assignment.id
})
run_transaction_commit_callbacks
@tag.update_asset_name!
@tag.reload
@ -276,7 +288,9 @@ describe ContentTag do
@tag.skip_touch = true
@tag.save
run_transaction_commit_callbacks
@module.reload.updated_at.to_i.should == yesterday.to_i
end
end