20472: Add a few comments and add container_tree function

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/services/api/db/migrate/20230503224107_priority_update_functions.rb b/services/api/db/migrate/20230503224107_priority_update_functions.rb
index dc9717f67f49de54afd18d8cd5a339b38f4ae897..df6f9d0bb53adfd3bcfd4b51ba6c8cd0c6dd4651 100644 (file)
--- a/services/api/db/migrate/20230503224107_priority_update_functions.rb
+++ b/services/api/db/migrate/20230503224107_priority_update_functions.rb
@@ -8,6 +8,10 @@ class PriorityUpdateFunctions < ActiveRecord::Migration[5.2]
 CREATE OR REPLACE FUNCTION container_priority(for_container_uuid character varying, inherited bigint, inherited_from character varying) returns bigint
     LANGUAGE sql
     AS $$
+/* Determine the priority of an individual container.
+   The "inherited" priority comes from the path we followed from the root, the parent container
+   priority hasn't been updated in the table yet but we need to behave it like it has been.
+*/
 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
@@ -22,6 +26,10 @@ $$;
 CREATE OR REPLACE FUNCTION update_priorities(for_container_uuid character varying) returns table (pri_container_uuid character varying, upd_priority bigint)
     LANGUAGE sql
     AS $$
+/* Calculate the priorities of all containers starting from for_container_uuid.
+   This traverses the process tree downward and calls container_priority for each container
+   and returns a table of container uuids and their new priorities.
+*/
 with recursive tab(upd_container_uuid, upd_priority) as (
   select for_container_uuid, container_priority(for_container_uuid, 0, '')
 union
@@ -32,11 +40,31 @@ union
 )
 select upd_container_uuid, upd_priority from tab;
 $$;
+}
+
+    ActiveRecord::Base.connection.execute %{
+CREATE OR REPLACE FUNCTION container_tree(for_container_uuid character varying) returns table (pri_container_uuid character varying)
+    LANGUAGE sql
+    AS $$
+/* A lighter weight version of the update_priorities query that only returns the containers in a tree,
+   used by SELECT FOR UPDATE.
+*/
+with recursive tab(upd_container_uuid) as (
+  select for_container_uuid
+union
+  select containers.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 from tab;
+$$;
 }
   end
 
   def down
     ActiveRecord::Base.connection.execute "DROP FUNCTION container_priority"
     ActiveRecord::Base.connection.execute "DROP FUNCTION update_priorities"
+    ActiveRecord::Base.connection.execute "DROP FUNCTION container_tree"
   end
 end

diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 1b0eab5f67994bd628807c5ba9a33030278689c2..ffb581baf3f208760ddeb3772552f96f9ad43e5d 100644 (file)
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -197,6 +197,10 @@ $$;
 CREATE FUNCTION public.container_priority(for_container_uuid character varying, inherited bigint, inherited_from character varying) RETURNS bigint
     LANGUAGE sql
     AS $$
+/* Determine the priority of an individual container.
+   The "inherited" priority comes from the path we followed from the root, the parent container
+   priority hasn't been updated in the table yet but we need to behave it like it has been.
+*/
 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
@@ -207,6 +211,28 @@ select coalesce(max(case when container_requests.priority = 0 then 0
 $$;
 
 
+--
+-- Name: container_tree(character varying); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.container_tree(for_container_uuid character varying) RETURNS TABLE(pri_container_uuid character varying)
+    LANGUAGE sql
+    AS $$
+/* A lighter weight version of the update_priorities query that only returns the containers in a tree,
+   used by SELECT FOR UPDATE.
+*/
+with recursive tab(upd_container_uuid) as (
+  select for_container_uuid
+union
+  select containers.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 from tab;
+$$;
+
+
 --
 -- Name: project_subtree_with_is_frozen(character varying, boolean); Type: FUNCTION; Schema: public; Owner: -
 --
@@ -276,6 +302,10 @@ $$;
 CREATE FUNCTION public.update_priorities(for_container_uuid character varying) RETURNS TABLE(pri_container_uuid character varying, upd_priority bigint)
     LANGUAGE sql
     AS $$
+/* Calculate the priorities of all containers starting from for_container_uuid.
+   This traverses the process tree downward and calls container_priority for each container
+   and returns a table of container uuids and their new priorities.
+*/
 with recursive tab(upd_container_uuid, upd_priority) as (
   select for_container_uuid, container_priority(for_container_uuid, 0, '')
 union

diff --git a/services/api/lib/update_priorities.rb b/services/api/lib/update_priorities.rb
index bd5b71b90cd72447d932238679c3dab5392049ca..b7799cb4bd056632a38aa9ded363e1e3b0cc4c98 100644 (file)
--- a/services/api/lib/update_priorities.rb
+++ b/services/api/lib/update_priorities.rb
@@ -5,12 +5,12 @@
 def update_priorities starting_container_uuid
   ActiveRecord::Base.connection.exec_query %{
 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
+ where containers.uuid = computed.pri_container_uuid and priority != computed.upd_priority
 }, '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 1 from containers where containers.uuid in (select pri_container_uuid from container_tree($1)) order by containers.uuid for update
   }, 'select_for_update_priorities', [[nil, container_uuid]]
 end