3859: Implement Job lock method on api server. This takes a queued job and

uses a transaction to set it as running without creating a race condition.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index d8ceb850f8a7c0dc7e1290b4b39f66f48961671b..2141d3375c428e5dadf536c2cb832e35f620c5a4 100644 (file)
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -100,6 +100,11 @@ class Arvados::V1::JobsController < ApplicationController
     show
   end
 
+  def lock
+    @object.lock current_user.uuid
+    show
+  end
+
   class LogStreamer
     Q_UPDATE_INTERVAL = 12
     def initialize(job, opts={})

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 376df0cef3cd9aea812bcdf1a92fff92b76f8952..2a98591a36ee10bae8826ff492840c7544f7a40f 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -42,6 +42,13 @@ class ArvadosModel < ActiveRecord::Base
     end
   end
 
+  class ConflictError < StandardError
+    def http_status
+      409
+    end
+  end
+
+
   def self.kind_class(kind)
     kind.match(/^arvados\#(.+)$/)[1].classify.safe_constantize rescue nil
   end

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 7da6852ee0143940866db0961641caf0015a202b..81744c7643bdf23a246f83d2adf7ae899ab24334 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -14,6 +14,7 @@ class Job < ArvadosModel
   validate :ensure_script_version_is_commit
   validate :find_docker_image_locator
   validate :validate_status
+  validate :validate_state_change
 
   has_many :commit_ancestors, :foreign_key => :descendant, :primary_key => :script_version
   has_many(:nodes, foreign_key: :job_uuid, primary_key: :uuid)
@@ -90,6 +91,18 @@ class Job < ArvadosModel
       order('priority desc, created_at')
   end
 
+  def lock locked_by_uuid
+    transaction do
+      self.reload
+      unless self.state == Queued and self.is_locked_by_uuid.nil?
+        raise ConflictError.new
+      end
+      self.state = Running
+      self.is_locked_by_uuid = locked_by_uuid
+      self.save!
+    end
+  end
+
   protected
 
   def foreign_key_attributes
@@ -112,7 +125,7 @@ class Job < ArvadosModel
   end
 
   def ensure_script_version_is_commit
-    if self.is_locked_by_uuid and self.started_at
+    if self.state == Running
       # Apparently client has already decided to go for it. This is
       # needed to run a local job using a local working directory
       # instead of a commit-ish.
@@ -199,7 +212,8 @@ class Job < ArvadosModel
           success_changed? or
           output_changed? or
           log_changed? or
-          tasks_summary_changed?
+          tasks_summary_changed? or
+          state_changed?
         logger.warn "User #{current_user.uuid if current_user} tried to change protected job attributes on locked #{self.class.to_s} #{uuid_was}"
         return false
       end
@@ -317,4 +331,18 @@ class Job < ArvadosModel
     end
   end
 
+  def validate_state_change
+    if self.state_changed?
+      if self.state_was.in? [Complete, Failed, Cancelled]
+        # Once in a finished state, don't permit any changes
+        errors.add :state, "invalid change from #{self.state_was} to #{self.state}"
+        return false
+      elsif self.state_was == Running and not self.state.in? [Complete, Failed, Cancelled]
+        # From running, can only transition to a finished state
+        errors.add :state, "invalid change from #{self.state_was} to #{self.state}"
+        return false
+      end
+    end
+    true
+  end
 end

diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index bcfe9b863638798cac34c38e9ee15a4b53a1bce2..705822a6a58eb507e2f42261364293e54494dc65 100644 (file)
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -24,6 +24,7 @@ Server::Application.routes.draw do
         get 'queue', on: :collection
         get 'queue_size', on: :collection
         post 'cancel', on: :member
+        post 'lock', on: :member
       end
       resources :keep_disks do
         post 'ping', on: :collection