From 57049b55a22ac0f2a339724410e1c7fa0be361d5 Mon Sep 17 00:00:00 2001 From: Robin Ward <robin.ward@gmail.com> Date: Mon, 11 Feb 2013 15:47:28 -0500 Subject: [PATCH] Little things: - Retries on deadlock when calculating average time - Removes Warning: When specifying html format for errors - Doesn't use manual SQL to update user's ip address --- app/controllers/application_controller.rb | 4 ++-- app/models/post.rb | 28 ++++++++++++----------- lib/current_user.rb | 2 +- lib/freedom_patches/active_record_base.rb | 18 +++++++++++++++ 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 128320a33..db118b323 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -66,12 +66,12 @@ class ApplicationController < ActionController::Base # for now do a simple remap, we may look at cleaner ways of doing the render raise ActiveRecord::RecordNotFound else - render file: 'public/404.html', layout: false, status: 404 + render file: 'public/404', formats: [:html], layout: false, status: 404 end end rescue_from Discourse::InvalidAccess do - render file: 'public/403.html', layout: false, status: 403 + render file: 'public/403', formats: [:html], layout: false, status: 403 end def store_preloaded(key, json) diff --git a/app/models/post.rb b/app/models/post.rb index 066cac96b..496224570 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -342,19 +342,21 @@ class Post < ActiveRecord::Base # This calculates the geometric mean of the post timings and stores it along with # each post. def self.calculate_avg_time - exec_sql("UPDATE posts - SET avg_time = (x.gmean / 1000) - FROM (SELECT post_timings.topic_id, - post_timings.post_number, - round(exp(avg(ln(msecs)))) AS gmean - FROM post_timings - INNER JOIN posts AS p2 - ON p2.post_number = post_timings.post_number - AND p2.topic_id = post_timings.topic_id - AND p2.user_id <> post_timings.user_id - GROUP BY post_timings.topic_id, post_timings.post_number) AS x - WHERE x.topic_id = posts.topic_id - AND x.post_number = posts.post_number") + retry_lock_error do + exec_sql("UPDATE posts + SET avg_time = (x.gmean / 1000) + FROM (SELECT post_timings.topic_id, + post_timings.post_number, + round(exp(avg(ln(msecs)))) AS gmean + FROM post_timings + INNER JOIN posts AS p2 + ON p2.post_number = post_timings.post_number + AND p2.topic_id = post_timings.topic_id + AND p2.user_id <> post_timings.user_id + GROUP BY post_timings.topic_id, post_timings.post_number) AS x + WHERE x.topic_id = posts.topic_id + AND x.post_number = posts.post_number") + end end before_save do diff --git a/lib/current_user.rb b/lib/current_user.rb index aa73a963a..89d47cc4f 100644 --- a/lib/current_user.rb +++ b/lib/current_user.rb @@ -23,7 +23,7 @@ module CurrentUser @current_user.update_last_seen! if @current_user.ip_address != request.remote_ip @current_user.ip_address = request.remote_ip - User.exec_sql('update users set ip_address = ? where id = ?', request.remote_ip, @current_user.id) + @current_user.update_column(:ip_address, request.remote_ip) end end @current_user diff --git a/lib/freedom_patches/active_record_base.rb b/lib/freedom_patches/active_record_base.rb index 110e2b566..41ae0e917 100644 --- a/lib/freedom_patches/active_record_base.rb +++ b/lib/freedom_patches/active_record_base.rb @@ -15,6 +15,24 @@ class ActiveRecord::Base ActiveRecord::Base.exec_sql(*args) end + + # Executes the given block +retries+ times (or forever, if explicitly given nil), + # catching and retrying SQL Deadlock errors. + # + # Thanks to: http://stackoverflow.com/a/7427186/165668 + # + def self.retry_lock_error(retries=5, &block) + begin + yield + rescue ActiveRecord::StatementInvalid => e + if e.message =~ /Deadlock found when trying to get lock/ and (retries.nil? || retries > 0) + retry_lock_error(retries ? retries - 1 : nil, &block) + else + raise e + end + end + end + # Support for psql. If we want to support multiple RDBMs in the future we can # split this. def exec_sql_row_count(*args)