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
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
include DbCurrentTime
include ContainerTestHelper
+ def with_container_auth(ctr)
+ auth_was = Thread.current[:api_client_authorization]
+ Thread.current[:api_client_authorization] = ApiClientAuthorization.find_by_uuid(ctr.auth_uuid)
+ begin
+ yield
+ ensure
+ Thread.current[:api_client_authorization] = auth_was
+ end
+ end
+
+ def lock_and_run(ctr)
+ act_as_system_user do
+ ctr.update_attributes!(state: Container::Locked)
+ ctr.update_attributes!(state: Container::Running)
+ end
+ end
+
def create_minimal_req! attrs={}
defaults = {
command: ["echo", "foo"],
assert_equal({"/out" => {"kind"=>"tmp", "capacity"=>1000000}}, c.mounts)
assert_equal "/out", c.output_path
assert_equal({"keep_cache_ram"=>268435456, "vcpus" => 2, "ram" => 30}, c.runtime_constraints)
- assert_equal 1, c.priority
+ assert_operator 0, :<, c.priority
assert_raises(ActiveRecord::RecordInvalid) do
cr.priority = nil
assert_equal 0, c.priority
end
-
- test "Container request max priority" do
- set_user_from_auth :active
- cr = create_minimal_req!(priority: 5, state: "Committed")
-
- c = Container.find_by_uuid cr.container_uuid
- assert_equal 5, c.priority
-
- cr2 = create_minimal_req!
- cr2.priority = 10
- cr2.state = "Committed"
- cr2.container_uuid = cr.container_uuid
- act_as_system_user do
- cr2.save!
- end
-
- # cr and cr2 have priority 5 and 10, and are being satisfied by
- # the same container c, so c's priority should be
- # max(priority)=10.
- c.reload
- assert_equal 10, c.priority
-
- cr2.update_attributes!(priority: 0)
-
- c.reload
- assert_equal 5, c.priority
-
- cr.update_attributes!(priority: 0)
-
- c.reload
- assert_equal 0, c.priority
- end
-
-
test "Independent container requests" do
set_user_from_auth :active
cr1 = create_minimal_req!(command: ["foo", "1"], priority: 5, state: "Committed")
cr2 = create_minimal_req!(command: ["foo", "2"], priority: 10, state: "Committed")
c1 = Container.find_by_uuid cr1.container_uuid
- assert_equal 5, c1.priority
+ assert_operator 0, :<, c1.priority
c2 = Container.find_by_uuid cr2.container_uuid
- assert_equal 10, c2.priority
+ assert_operator c1.priority, :<, c2.priority
+ c2priority_was = c2.priority
cr1.update_attributes!(priority: 0)
assert_equal 0, c1.priority
c2.reload
- assert_equal 10, c2.priority
+ assert_equal c2priority_was, c2.priority
end
test "Request is finalized when its container is cancelled" do
cr = create_minimal_req!(priority: 5, state: "Committed", container_count_max: 1)
c = Container.find_by_uuid cr.container_uuid
- assert_equal 5, c.priority
+ assert_operator 0, :<, c.priority
cr2 = create_minimal_req!
cr2.update_attributes!(priority: 10, state: "Committed", requesting_container_uuid: c.uuid, command: ["echo", "foo2"], container_count_max: 1)
cr2.reload
c2 = Container.find_by_uuid cr2.container_uuid
- assert_equal 10, c2.priority
+ assert_operator 0, :<, c2.priority
act_as_system_user do
c.state = "Cancelled"
assert_equal 0, c2.priority
end
+ test "child container priority follows same ordering as corresponding top-level ancestors" do
+ findctr = lambda { |cr| Container.find_by_uuid(cr.container_uuid) }
- test "Container makes container request, then changes priority" do
set_user_from_auth :active
- cr = create_minimal_req!(priority: 5, state: "Committed", container_count_max: 1)
- c = Container.find_by_uuid cr.container_uuid
- assert_equal 5, c.priority
-
- cr2 = create_minimal_req!
- cr2.update_attributes!(priority: 5, state: "Committed", requesting_container_uuid: c.uuid, command: ["echo", "foo2"], container_count_max: 1)
- cr2.reload
-
- c2 = Container.find_by_uuid cr2.container_uuid
- assert_equal 5, c2.priority
+ toplevel_crs = [
+ create_minimal_req!(priority: 5, state: "Committed", environment: {"workflow" => "0"}),
+ create_minimal_req!(priority: 5, state: "Committed", environment: {"workflow" => "1"}),
+ create_minimal_req!(priority: 5, state: "Committed", environment: {"workflow" => "2"}),
+ ]
+ parents = toplevel_crs.map(&findctr)
+
+ children = parents.map do |parent|
+ lock_and_run(parent)
+ with_container_auth(parent) do
+ create_minimal_req!(state: "Committed",
+ priority: 1,
+ environment: {"child" => parent.environment["workflow"]})
+ end
+ end.map(&findctr)
+
+ grandchildren = children.reverse.map do |child|
+ lock_and_run(child)
+ with_container_auth(child) do
+ create_minimal_req!(state: "Committed",
+ priority: 1,
+ environment: {"grandchild" => child.environment["child"]})
+ end
+ end.reverse.map(&findctr)
- act_as_system_user do
- c.priority = 10
- c.save!
- end
+ shared_grandchildren = children.map do |child|
+ with_container_auth(child) do
+ create_minimal_req!(state: "Committed",
+ priority: 1,
+ environment: {"grandchild" => "shared"})
+ end
+ end.map(&findctr)
- cr.reload
+ assert_equal shared_grandchildren[0].uuid, shared_grandchildren[1].uuid
+ assert_equal shared_grandchildren[0].uuid, shared_grandchildren[2].uuid
+ shared_grandchild = shared_grandchildren[0]
- cr2.reload
- assert_equal 10, cr2.priority
+ set_user_from_auth :active
- c2.reload
- assert_equal 10, c2.priority
+ # parents should be prioritized by submit time.
+ assert_operator children[0].priority, :>, children[1].priority
+ assert_operator children[1].priority, :>, children[2].priority
+
+ # children should be prioritized in same order as their respective
+ # parents.
+ assert_operator children[0].priority, :>, children[1].priority
+ assert_operator children[1].priority, :>, children[2].priority
+
+ # grandchildren should also be prioritized in the same order,
+ # despite having been submitted in the opposite order.
+ assert_operator grandchildren[0].priority, :>, grandchildren[1].priority
+ assert_operator grandchildren[1].priority, :>, grandchildren[2].priority
+
+ # shared grandchild container should be prioritized above
+ # everything that isn't needed by parents[0], but not above
+ # earlier-submitted descendants of parents[0]
+ assert_operator shared_grandchild.priority, :>, grandchildren[1].priority
+ assert_operator shared_grandchild.priority, :>, children[1].priority
+ assert_operator shared_grandchild.priority, :>, parents[1].priority
+ assert_operator shared_grandchild.priority, :<=, grandchildren[0].priority
+ assert_operator shared_grandchild.priority, :<=, children[0].priority
+ assert_operator shared_grandchild.priority, :<=, parents[0].priority
+
+ # increasing priority of the most recent toplevel container should
+ # reprioritize all of its descendants (including the shared
+ # grandchild) above everything else.
+ toplevel_crs[2].update_attributes!(priority: 72)
+ (parents + children + grandchildren + [shared_grandchild]).map(&:reload)
+ assert_operator shared_grandchild.priority, :>, grandchildren[0].priority
+ assert_operator shared_grandchild.priority, :>, children[0].priority
+ assert_operator shared_grandchild.priority, :>, parents[0].priority
+ assert_operator shared_grandchild.priority, :>, grandchildren[1].priority
+ assert_operator shared_grandchild.priority, :>, children[1].priority
+ assert_operator shared_grandchild.priority, :>, parents[1].priority
+ # ...but the shared container should not have higher priority than
+ # the earlier-submitted descendants of the high-priority workflow.
+ assert_operator shared_grandchild.priority, :<=, grandchildren[2].priority
+ assert_operator shared_grandchild.priority, :<=, children[2].priority
+ assert_operator shared_grandchild.priority, :<=, parents[2].priority
end
[
- ['running_container_auth', 'zzzzz-dz642-runningcontainr', 12],
+ ['running_container_auth', 'zzzzz-dz642-runningcontainr', 1],
['active_no_prefs', nil, 0],
].each do |token, expected, expected_priority|
test "create as #{token} and expect requesting_container_uuid to be #{expected}" do
c = Container.find_by_uuid(cr.container_uuid)
assert_equal 1, c.priority
- # destroy the cr
- assert_nothing_raised {cr.destroy}
+ cr.destroy
# the cr's container now has priority of 0
c = Container.find_by_uuid(cr.container_uuid)