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.

1 files changed, 13 insertions, 0 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