Merge branch '15964-fix-docs' refs #15964

[arvados.git] / services / api / lib / update_priority.rb

diff --git a/services/api/lib/update_priority.rb b/services/api/lib/update_priority.rb
index 21cd74bae67d3cbf94257284aedf05ab20aaad57..6c17f1bd03bf5bae3d1dbd9a2a9e4123ee99b715 100644 (file)
--- a/services/api/lib/update_priority.rb
+++ b/services/api/lib/update_priority.rb
@@ -12,14 +12,19 @@ module UpdatePriority
   #
   # If container priority=0 but there are committed container requests
   # for it with priority>0, update priority.
-  def self.update_priority
+  #
+  # Normally, update_priority is a no-op if another thread/process is
+  # already updating. Test cases that need to check priorities after
+  # updating can force a (possibly overlapping) update in the current
+  # thread/transaction by setting the "nolock" flag. See #14878.
+  def self.update_priority(nolock: false)
     if !File.owned?(Rails.root.join('tmp'))
       Rails.logger.warn("UpdatePriority: not owner of #{Rails.root}/tmp, skipping")
       return
     end
     lockfile = Rails.root.join('tmp', 'update_priority.lock')
     File.open(lockfile, File::RDWR|File::CREAT, 0600) do |f|
-      return unless f.flock(File::LOCK_NB|File::LOCK_EX)
+      return unless nolock || f.flock(File::LOCK_NB|File::LOCK_EX)
 
       # priority>0 but should be 0:
       ActiveRecord::Base.connection.
@@ -28,7 +33,7 @@ module UpdatePriority
       # priority==0 but should be >0:
       act_as_system_user do
         Container.
-          joins("JOIN container_requests ON container_requests.container_uuid=containers.uuid AND container_requests.state=#{Container.sanitize(ContainerRequest::Committed)} AND container_requests.priority>0").
+          joins("JOIN container_requests ON container_requests.container_uuid=containers.uuid AND container_requests.state=#{ActiveRecord::Base.connection.quote(ContainerRequest::Committed)} AND container_requests.priority>0").
           where('containers.state IN (?) AND containers.priority=0 AND container_requests.uuid IS NOT NULL',
                 [Container::Queued, Container::Locked, Container::Running]).
           map(&:update_priority!)
@@ -50,7 +55,12 @@ module UpdatePriority
       rescue => e
         Rails.logger.error "#{e.class}: #{e}\n#{e.backtrace.join("\n\t")}"
       ensure
-        ActiveRecord::Base.connection.close
+        # Rails 5.1+ makes test threads share a database connection, so we can't
+        # close a connection shared with other threads.
+        # https://github.com/rails/rails/commit/deba47799ff905f778e0c98a015789a1327d5087
+        if Rails.env != "test"
+          ActiveRecord::Base.connection.close
+        end
       end
     end
   end