[arvados] updated: 2.6.0-98-g1c7e8986e
git repository hosting
git at public.arvados.org
Thu May 4 01:26:55 UTC 2023
Summary of changes:
.../arvados/v1/container_requests_controller.rb | 23 ------------
.../arvados/v1/containers_controller.rb | 26 ++------------
services/api/app/models/container.rb | 31 ++--------------
services/api/app/models/container_request.rb | 7 ++--
.../20230503224107_priority_update_functions.rb | 41 ++++++++++++++++++++++
services/api/lib/update_priorities.rb | 10 ++++++
6 files changed, 57 insertions(+), 81 deletions(-)
create mode 100644 services/api/db/migrate/20230503224107_priority_update_functions.rb
create mode 100644 services/api/lib/update_priorities.rb
via 1c7e8986e9a71b693e65f91ce0f3482260c5c96b (commit)
via 62a850805eed2176f592f7e769061c3f692b83ab (commit)
from ff843a40de882d22dd2a5e408c77a2fa4720cc7d (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 1c7e8986e9a71b693e65f91ce0f3482260c5c96b
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 3 21:17:27 2023 -0400
20470: Remove locks on containers table
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/controllers/arvados/v1/container_requests_controller.rb b/services/api/app/controllers/arvados/v1/container_requests_controller.rb
index 586567cb2..afddd5686 100644
--- a/services/api/app/controllers/arvados/v1/container_requests_controller.rb
+++ b/services/api/app/controllers/arvados/v1/container_requests_controller.rb
@@ -29,27 +29,4 @@ class Arvados::V1::ContainerRequestsController < ApplicationController
})
end
- def create
- # Lock containers table to avoid deadlock in cascading priority update (see #20240)
- Container.transaction do
- ActiveRecord::Base.connection.execute "LOCK TABLE containers IN EXCLUSIVE MODE"
- super
- end
- end
-
- def update
- if (resource_attrs.keys - [:owner_uuid, :name, :description, :properties]).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.
- @object.reload(lock: true)
- super
- else
- # Lock containers table to avoid deadlock in cascading priority update (see #20240)
- Container.transaction do
- ActiveRecord::Base.connection.execute "LOCK TABLE containers IN EXCLUSIVE MODE"
- super
- end
- end
- end
end
diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index 83f99bf92..b7dc03022 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -28,23 +28,6 @@ class Arvados::V1::ContainersController < ApplicationController
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.
- @object.reload(lock: true)
- super
- else
- # Lock containers table to avoid deadlock in cascading priority
- # update (see #20240)
- Container.transaction do
- ActiveRecord::Base.connection.execute "LOCK TABLE containers IN EXCLUSIVE MODE"
- super
- end
- end
- end
-
def find_objects_for_index
super
if action_name == 'lock' || action_name == 'unlock'
@@ -70,13 +53,8 @@ class Arvados::V1::ContainersController < ApplicationController
end
def update_priority
- # Lock containers table to avoid deadlock in cascading priority update (see #20240)
- Container.transaction do
- ActiveRecord::Base.connection.execute "LOCK TABLE containers IN EXCLUSIVE MODE"
- @object.reload(lock: true)
- @object.update_priority!
- show
- end
+ @object.update_priority!
+ show
end
def current
commit 62a850805eed2176f592f7e769061c3f692b83ab
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 3 21:13:06 2023 -0400
20470: Update priorities with a single stored query
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 61557eacb..6301fe808 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -5,6 +5,7 @@
require 'log_reuse_info'
require 'whitelist_update'
require 'safe_json'
+require 'update_priorities'
class Container < ArvadosModel
include ArvadosModelUpdates
@@ -49,7 +50,6 @@ class Container < ArvadosModel
before_save :clear_runtime_status_when_queued
after_save :update_cr_logs
after_save :handle_completed
- after_save :propagate_priority
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
@@ -129,34 +129,7 @@ class Container < ArvadosModel
# priority of a user-submitted request is a function of
# user-assigned priority and request creation time.
def update_priority!
- return if ![Queued, Locked, Running].include?(state)
- p = ContainerRequest.
- where('container_uuid=? and priority>0 and state=?', uuid, ContainerRequest::Committed).
- select("priority, requesting_container_uuid, created_at").
- lock(true).
- map do |cr|
- if cr.requesting_container_uuid
- Container.where(uuid: cr.requesting_container_uuid).pluck(:priority).first
- 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
- return true unless saved_change_to_priority?
- 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 = ? and state = ? and container_uuid is not null',
- self.uuid, ContainerRequest::Committed).
- pluck(:container_uuid).each do |container_uuid|
- Container.find_by_uuid(container_uuid).update_priority!
- end
- end
+ update_priorities uuid
end
# Create a new container (or find an existing one) to satisfy the
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 09da141ea..3c3896771 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -562,11 +562,8 @@ class ContainerRequest < ArvadosModel
def update_priority
return unless saved_change_to_state? || saved_change_to_priority? || saved_change_to_container_uuid?
- act_as_system_user do
- Container.
- where('uuid in (?)', [container_uuid_before_last_save, self.container_uuid].compact).
- map(&:update_priority!)
- end
+ update_priorities container_uuid_before_last_save if !container_uuid_before_last_save.nil? and container_uuid_before_last_save != self.container_uuid
+ update_priorities self.container_uuid if self.container_uuid
end
def set_requesting_container_uuid
diff --git a/services/api/db/migrate/20230503224107_priority_update_functions.rb b/services/api/db/migrate/20230503224107_priority_update_functions.rb
new file mode 100644
index 000000000..fbb3e27bf
--- /dev/null
+++ b/services/api/db/migrate/20230503224107_priority_update_functions.rb
@@ -0,0 +1,41 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+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
+ 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)
+ 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;
+$$;
+}
+
+ ActiveRecord::Base.connection.execute %{
+CREATE OR REPLACE FUNCTION update_priorities(for_container_uuid character varying) returns table (pri_container_uuid character varying, 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)
+ 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;
+$$;
+}
+ end
+
+ def down
+ ActiveRecord::Base.connection.execute "DROP FUNCTION container_priority"
+ ActiveRecord::Base.connection.execute "DROP FUNCTION update_priorities"
+ end
+end
diff --git a/services/api/lib/update_priorities.rb b/services/api/lib/update_priorities.rb
new file mode 100644
index 000000000..66d307436
--- /dev/null
+++ b/services/api/lib/update_priorities.rb
@@ -0,0 +1,10 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+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
+ where containers.uuid = computed.pri_container_uuid
+}, 'update_priorities', [[nil, starting_container_uuid]]
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list