X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/01126d2f7b8a9134615e0a5f0c933622750db937..0eedf70afa34167275d2135837e866b13fac4178:/services/api/app/models/container.rb diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index 2bb71c3ee7..7ec9845bc1 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -5,8 +5,10 @@ require 'log_reuse_info' require 'whitelist_update' require 'safe_json' +require 'update_priority' class Container < ArvadosModel + include ArvadosModelUpdates include HasUuid include KindAndEtag include CommonApiTemplate @@ -24,8 +26,8 @@ class Container < ArvadosModel before_validation :fill_field_defaults, :if => :new_record? before_validation :set_timestamps - validates :command, :container_image, :output_path, :cwd, :priority, :presence => true - validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 } + validates :command, :container_image, :output_path, :cwd, :priority, { presence: true } + validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0 } validate :validate_state_change validate :validate_change validate :validate_lock @@ -36,6 +38,7 @@ class Container < ArvadosModel before_save :scrub_secret_mounts after_save :handle_completed after_save :propagate_priority + after_commit { UpdatePriority.run_update_thread } has_many :container_requests, :foreign_key => :container_uuid, :class_name => 'ContainerRequest', :primary_key => :uuid belongs_to :auth, :class_name => 'ApiClientAuthorization', :foreign_key => :auth_uuid, :primary_key => :uuid @@ -98,29 +101,40 @@ class Container < ArvadosModel State_transitions end + # Container priority is the highest "computed priority" of any + # matching request. The computed priority of a container-submitted + # request is the priority of the submitting container. The computed + # priority of a user-submitted request is a function of + # user-assigned priority and request creation time. def update_priority! - if [Queued, Locked, Running].include? self.state - # Update the priority of this container to the maximum priority of any of - # its committed container requests and save the record. - self.priority = ContainerRequest. - where(container_uuid: uuid, - state: ContainerRequest::Committed). - maximum('priority') || 0 - self.save! - end + return if ![Queued, Locked, Running].include?(state) + p = ContainerRequest. + where('container_uuid=? and priority>0', uuid). + includes(:requesting_container). + lock(true). + map do |cr| + if cr.requesting_container + cr.requesting_container.priority + else + (cr.priority << 50) - (cr.created_at.to_time.to_f * 1000).to_i + end + end.max || 0 + update_attributes!(priority: p) end def propagate_priority - if self.priority_changed? - act_as_system_user do - # Update the priority of child container requests to match new priority - # of the parent container. - ContainerRequest.where(requesting_container_uuid: self.uuid, - state: ContainerRequest::Committed).each do |cr| - cr.priority = self.priority - cr.save - end - end + return true unless priority_changed? + act_as_system_user do + # 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). + ContainerRequest. + where(requesting_container_uuid: self.uuid, + state: ContainerRequest::Committed). + where('container_uuid is not null'). + includes(:container). + map(&:container). + map(&:update_priority!) end end @@ -303,11 +317,7 @@ 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 + reload check_lock_fail update_attributes!(state: Locked) end @@ -549,9 +559,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 @@ -559,7 +571,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. @@ -567,19 +581,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