#
# SPDX-License-Identifier: AGPL-3.0
+require 'update_priorities'
+
class Arvados::V1::ContainerRequestsController < ApplicationController
accept_attribute_as_json :environment, Hash
accept_attribute_as_json :mounts, Hash
})
end
+ def update
+ if (resource_attrs.keys - [:owner_uuid, :name, :description, :properties]).empty? or @object.container_uuid.nil?
+ # If no attributes are being updated besides these, there are no
+ # cascading changes to other rows/tables, so we should just use
+ # row locking.
+ super
+ else
+ # Lock containers table to avoid deadlock in cascading priority update (see #20240)
+ Container.transaction do
+ row_lock_for_priority_update @object.container_uuid
+ super
+ end
+ end
+ end
end
#
# SPDX-License-Identifier: AGPL-3.0
+require 'update_priorities'
+
class Arvados::V1::ContainersController < ApplicationController
accept_attribute_as_json :environment, Hash
accept_attribute_as_json :mounts, Hash
show
end
+ def update
+ if (resource_attrs.keys - [:cost, :gateway_address, :output_properties, :progress, :runtime_status]).empty?
+ # If no attributes are being updated besides these, there are no
+ # cascading changes to other rows/tables, so we should just use
+ # row locking.
+ super
+ else
+ Container.transaction do
+ # Get locks ahead of time to avoid deadlock in cascading priority
+ # update
+ row_lock_for_priority_update @object.uuid
+ super
+ end
+ end
+ end
+
def find_objects_for_index
super
if action_name == 'lock' || action_name == 'unlock'
# user-assigned priority and request creation time.
def update_priority!
update_priorities uuid
+ reload
end
# Create a new container (or find an existing one) to satisfy the
class PriorityUpdateFunctions < ActiveRecord::Migration[5.2]
def up
ActiveRecord::Base.connection.execute %{
-CREATE OR REPLACE FUNCTION container_priority(for_container_uuid character varying, inherited bigint) returns bigint
+CREATE OR REPLACE FUNCTION container_priority(for_container_uuid character varying, inherited bigint, inherited_from character varying) returns bigint
LANGUAGE sql
AS $$
select coalesce(max(case when container_requests.priority = 0 then 0
- when containers.priority is not NULL then greatest(containers.priority, inherited)
+ when containers.uuid = inherited_from then inherited
+ when containers.priority is not NULL then containers.priority
else container_requests.priority * 1125899906842624::bigint - (extract(epoch from container_requests.created_at)*1000)::bigint
end), 0) from
container_requests left outer join containers on container_requests.requesting_container_uuid = containers.uuid
}
ActiveRecord::Base.connection.execute %{
-CREATE OR REPLACE FUNCTION update_priorities(for_container_uuid character varying) returns table (pri_container_uuid character varying, priority bigint)
+CREATE OR REPLACE FUNCTION update_priorities(for_container_uuid character varying) returns table (pri_container_uuid character varying, upd_priority bigint)
LANGUAGE sql
AS $$
with recursive tab(upd_container_uuid, upd_priority) as (
- select for_container_uuid, container_priority(for_container_uuid, 0)
+ select for_container_uuid, container_priority(for_container_uuid, 0, '')
union
- select containers.uuid, container_priority(containers.uuid, child_requests.upd_priority)
+ select containers.uuid, container_priority(containers.uuid, child_requests.upd_priority, child_requests.upd_container_uuid)
from (tab join container_requests on tab.upd_container_uuid = container_requests.requesting_container_uuid) as child_requests
join containers on child_requests.container_uuid = containers.uuid
where containers.state in ('Queued', 'Locked', 'Running')
$$;
+--
+-- Name: container_priority(character varying, bigint, character varying); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.container_priority(for_container_uuid character varying, inherited bigint, inherited_from character varying) RETURNS bigint
+ LANGUAGE sql
+ AS $$
+select coalesce(max(case when container_requests.priority = 0 then 0
+ when containers.uuid = inherited_from then inherited
+ when containers.priority is not NULL then containers.priority
+ else container_requests.priority * 1125899906842624::bigint - (extract(epoch from container_requests.created_at)*1000)::bigint
+ end), 0) from
+ container_requests left outer join containers on container_requests.requesting_container_uuid = containers.uuid
+ where container_requests.container_uuid = for_container_uuid and container_requests.state = 'Committed' and container_requests.priority > 0;
+$$;
+
+
--
-- Name: project_subtree_with_is_frozen(character varying, boolean); Type: FUNCTION; Schema: public; Owner: -
--
$$;
+--
+-- Name: update_priorities(character varying); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.update_priorities(for_container_uuid character varying) RETURNS TABLE(pri_container_uuid character varying, upd_priority bigint)
+ LANGUAGE sql
+ AS $$
+with recursive tab(upd_container_uuid, upd_priority) as (
+ select for_container_uuid, container_priority(for_container_uuid, 0, '')
+union
+ select containers.uuid, container_priority(containers.uuid, child_requests.upd_priority, child_requests.upd_container_uuid)
+ from (tab join container_requests on tab.upd_container_uuid = container_requests.requesting_container_uuid) as child_requests
+ join containers on child_requests.container_uuid = containers.uuid
+ where containers.state in ('Queued', 'Locked', 'Running')
+)
+select upd_container_uuid, upd_priority from tab;
+$$;
+
+
SET default_tablespace = '';
+SET default_with_oids = false;
+
--
-- Name: api_client_authorizations; Type: TABLE; Schema: public; Owner: -
--
('20220804133317'),
('20221219165512'),
('20221230155924'),
-('20230421142716');
+('20230421142716'),
+('20230503224107');
def update_priorities starting_container_uuid
ActiveRecord::Base.connection.exec_query %{
-update containers set priority=computed.priority from (select pri_container_uuid, priority from update_priorities($1) order by pri_container_uuid) as computed
+update containers set priority=computed.upd_priority from (select pri_container_uuid, upd_priority from update_priorities($1) order by pri_container_uuid) as computed
where containers.uuid = computed.pri_container_uuid
}, 'update_priorities', [[nil, starting_container_uuid]]
end
+
+def row_lock_for_priority_update container_uuid
+ ActiveRecord::Base.connection.exec_query %{
+ select 1 from containers where containers.uuid in (select pri_container_uuid from update_priorities($1)) order by containers.uuid for update
+ }, 'select_for_update_priorities', [[nil, container_uuid]]
+end
assert_operator shared_grandchild.priority, :<=, grandchildren[2].priority
assert_operator shared_grandchild.priority, :<=, children[2].priority
assert_operator shared_grandchild.priority, :<=, parents[2].priority
+
+ # cancelling the most recent toplevel container should
+ # reprioritize all of its descendants (except the shared
+ # grandchild) to zero
+ toplevel_crs[2].update_attributes!(priority: 0)
+ (parents + children + grandchildren + [shared_grandchild]).map(&:reload)
+ assert_operator 0, :==, parents[2].priority
+ assert_operator 0, :==, children[2].priority
+ assert_operator 0, :==, grandchildren[2].priority
+ assert_operator shared_grandchild.priority, :==, grandchildren[0].priority
end
[