From 4671db01d21dad219582444592e99a74d1fa35d8 Mon Sep 17 00:00:00 2001
From: Peter Amstutz <peter.amstutz@curoverse.com>
Date: Thu, 25 Sep 2014 09:30:20 -0400
Subject: [PATCH] 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.

---
 .../controllers/arvados/v1/jobs_controller.rb |  5 +++
 services/api/app/models/arvados_model.rb      |  7 ++++
 services/api/app/models/job.rb                | 32 +++++++++++++++++--
 services/api/config/routes.rb                 |  1 +
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index d8ceb850f8..2141d3375c 100644
--- 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 376df0cef3..2a98591a36 100644
--- 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 7da6852ee0..81744c7643 100644
--- 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 bcfe9b8636..705822a6a5 100644
--- 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
-- 
2.30.2