Implement review comments from @DouweM and @nick.thomas.

1. Use an advisory lock to guarantee the absence of concurrency in `User.ghost`,
to prevent data races from creating more than one ghost, or preventing the
creation of ghost users by causing validation errors.

2. Use `update_all` instead of updating issues one-by-one.

2 files changed, 15 insertions, 2 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 13529c9997a..e4adcd6cde9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -351,6 +351,17 @@ class User < ActiveRecord::Base
 
       ghost_user ||
         begin
+          # Since we only want a single ghost user in an instance, we use an
+          # advisory lock to ensure than this block is never run concurrently.
+          advisory_lock = Gitlab::Database::AdvisoryLocking.new(:ghost_user)
+          advisory_lock.lock
+
+          # Recheck if a ghost user is already present (one might have been)
+          # added between the time we last checked (first line of this method)
+          # and the time we acquired the lock.
+          ghost_user = User.with_state(:ghost).first
+          return ghost_user if ghost_user.present?
+
           uniquify = Uniquify.new
 
           username = uniquify.string("ghost", -> (s) { User.find_by_username(s) })
@@ -364,6 +375,8 @@ class User < ActiveRecord::Base
             username: username, password: Devise.friendly_token,
             email: email, name: "Ghost User", state: :ghost
           )
+        ensure
+          advisory_lock.unlock
         end
     end
   end
diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb
index d88b81c04e2..bf1e49734cc 100644
--- a/app/services/users/destroy_service.rb
+++ b/app/services/users/destroy_service.rb
@@ -37,12 +37,12 @@ module Users
     end
 
     private
-
+    
     def move_issues_to_ghost_user(user)
       ghost_user = User.ghost
 
       Issue.transaction do
-        user.issues.each { |issue| issue.update!(author: ghost_user) }
+        user.issues.update_all(author_id: ghost_user.id)
       end
 
       user.reload