From 34ede909279fa865e1b45da2a515fd79a15edb42 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 2 Jan 2020 11:37:52 +1100 Subject: [PATCH] FIX: under rare conditions saving a new draft could error temporarily Under rare conditions due to bad HTTP timing and so on a draft could be set at the exact same time from 2 unicorn workers. When this happened and all the stars aligned, one of the sets would win and the other would raise an error. This transparently handles the situation without adding any cost to the draft system. The alternative is to add a distributed mutex, tricky DB transaction or handle the error in the controller. However this seems like a reasonable way to work around a pretty big edge case. --- app/models/draft.rb | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/app/models/draft.rb b/app/models/draft.rb index 4f7a03711f3..aee066c0e27 100644 --- a/app/models/draft.rb +++ b/app/models/draft.rb @@ -7,7 +7,7 @@ class Draft < ActiveRecord::Base class OutOfSequence < StandardError; end - def self.set(user, key, sequence, data, owner = nil) + def self.set(user, key, sequence, data, owner = nil, retry_not_unique: true) if SiteSetting.backup_drafts_to_pm_length > 0 && SiteSetting.backup_drafts_to_pm_length < data.length backup_draft(user, key, sequence, data) end @@ -65,13 +65,24 @@ class Draft < ActiveRecord::Base elsif sequence != current_sequence raise Draft::OutOfSequence else - Draft.create!( - user_id: user.id, - draft_key: key, - data: data, - sequence: sequence, - owner: owner - ) + begin + Draft.create!( + user_id: user.id, + draft_key: key, + data: data, + sequence: sequence, + owner: owner + ) + rescue ActiveRecord::RecordNotUnique => e + # we need this to be fast and with minimal locking, in some cases we can have a race condition + # around 2 controller actions calling for draft creation at the exact same time + # to avoid complex locking and a distributed mutex, since this is so rare, simply add a single retry + if retry_not_unique + set(user, key, sequence, data, owenr, retry_not_unique: false) + else + raise e + end + end end sequence