From 01006263b1715061b1d5719a55efc6a911bc3c90 Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Wed, 12 Apr 2017 09:33:54 +0800 Subject: [PATCH] prevent deadlocks by only locking the exact records we need to have locked - select ... for update not selecting on the primary key will cause more rows to be locked than intended, potentially causing deadlocks with concurrent uploads. - also rescue from any related exceptions. In the rare case that existing is found but deleted before it can be locked, a RecordNotFound will be thrown. Deduplication cannot happen in this case, just ignoring the error is fine. --- app/models/attachment.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 26fe4a8..89b9f8a 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -417,27 +417,33 @@ class Attachment < ActiveRecord::Base reused = with_lock do if existing = Attachment - .lock .where(digest: self.digest, filesize: self.filesize) .where('id <> ? and disk_filename <> ?', self.id, self.disk_filename) .first + existing.with_lock do - original_diskfile = self.diskfile - existing_diskfile = existing.diskfile + original_diskfile = self.diskfile + existing_diskfile = existing.diskfile - if File.readable?(original_diskfile) && - File.readable?(existing_diskfile) && - FileUtils.identical?(original_diskfile, existing_diskfile) + if File.readable?(original_diskfile) && + File.readable?(existing_diskfile) && + FileUtils.identical?(original_diskfile, existing_diskfile) - self.update_columns disk_directory: existing.disk_directory, - disk_filename: existing.disk_filename + self.update_columns disk_directory: existing.disk_directory, + disk_filename: existing.disk_filename + end end end end if reused File.delete(original_diskfile) end + rescue ActiveRecord::StatementInvalid, ActiveRecord::RecordNotFound + # Catch and ignore lock errors. It is not critical if deduplication does + # not happen, therefore we do not retry. + # with_lock throws ActiveRecord::RecordNotFound if the record isnt there + # anymore, thats why this is caught and ignored as well. end -- 2.1.4