9918: Merge branch 'master' into 9918-index-timeout

[arvados.git] / services / api / app / models / container.rb

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 3765266405d86f41a1a3a10372b79d1c17aea14f..1dbdb571050a70ec3a684f18f335269ac35fd6f8 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -7,6 +7,7 @@ require 'whitelist_update'
 require 'safe_json'
 
 class Container < ArvadosModel
+  include ArvadosModelUpdates
   include HasUuid
   include KindAndEtag
   include CommonApiTemplate
@@ -125,6 +126,7 @@ class Container < ArvadosModel
       # Update the priority of child container requests to match new
       # priority of the parent container (ignoring requests with no
       # container assigned, because their priority doesn't matter).
+      ActiveRecord::Base.connection.execute('LOCK container_requests, containers IN EXCLUSIVE MODE')
       ContainerRequest.
         where(requesting_container_uuid: self.uuid,
               state: ContainerRequest::Committed).
@@ -314,11 +316,11 @@ class Container < ArvadosModel
     # (because state might have changed while acquiring the lock).
     check_lock_fail
     transaction do
-      begin
-        reload(lock: 'FOR UPDATE NOWAIT')
-      rescue
-        raise LockFailedError.new("cannot lock: other transaction in progress")
-      end
+      # Locking involves assigning auth_uuid, which involves looking
+      # up container requests, so we must lock both tables in the
+      # proper order to avoid deadlock.
+      ActiveRecord::Base.connection.execute('LOCK container_requests, containers IN EXCLUSIVE MODE')
+      reload
       check_lock_fail
       update_attributes!(state: Locked)
     end
@@ -540,6 +542,7 @@ class Container < ArvadosModel
     if self.state_changed? and self.final?
       act_as_system_user do
 
+        ActiveRecord::Base.connection.execute('LOCK container_requests, containers IN EXCLUSIVE MODE')
         if self.state == Cancelled
           retryable_requests = ContainerRequest.where("container_uuid = ? and priority > 0 and state = 'Committed' and container_count < container_count_max", uuid)
         else
@@ -560,9 +563,11 @@ class Container < ArvadosModel
           c = Container.create! c_attrs
           retryable_requests.each do |cr|
             cr.with_lock do
-              # Use row locking because this increments container_count
-              cr.container_uuid = c.uuid
-              cr.save!
+              leave_modified_by_user_alone do
+                # Use row locking because this increments container_count
+                cr.container_uuid = c.uuid
+                cr.save!
+              end
             end
           end
         end
@@ -570,7 +575,9 @@ class Container < ArvadosModel
         # Notify container requests associated with this container
         ContainerRequest.where(container_uuid: uuid,
                                state: ContainerRequest::Committed).each do |cr|
-          cr.finalize!
+          leave_modified_by_user_alone do
+            cr.finalize!
+          end
         end
 
         # Cancel outstanding container requests made by this container.
@@ -578,19 +585,20 @@ class Container < ArvadosModel
           includes(:container).
           where(requesting_container_uuid: uuid,
                 state: ContainerRequest::Committed).each do |cr|
-          cr.update_attributes!(priority: 0)
-          cr.container.reload
-          if cr.container.state == Container::Queued || cr.container.state == Container::Locked
-            # If the child container hasn't started yet, finalize the
-            # child CR now instead of leaving it "on hold", i.e.,
-            # Queued with priority 0.  (OTOH, if the child is already
-            # running, leave it alone so it can get cancelled the
-            # usual way, get a copy of the log collection, etc.)
-            cr.update_attributes!(state: ContainerRequest::Final)
+          leave_modified_by_user_alone do
+            cr.update_attributes!(priority: 0)
+            cr.container.reload
+            if cr.container.state == Container::Queued || cr.container.state == Container::Locked
+              # If the child container hasn't started yet, finalize the
+              # child CR now instead of leaving it "on hold", i.e.,
+              # Queued with priority 0.  (OTOH, if the child is already
+              # running, leave it alone so it can get cancelled the
+              # usual way, get a copy of the log collection, etc.)
+              cr.update_attributes!(state: ContainerRequest::Final)
+            end
           end
         end
       end
     end
   end
-
 end