[arvados] updated: 2.1.0-3160-g42dd3d670

git repository hosting git at public.arvados.org
Wed Dec 14 14:24:09 UTC 2022


Summary of changes:
 lib/config/config.default.yml                    | 12 +++++-----
 lib/config/export.go                             |  1 -
 sdk/cwl/arvados_cwl/arvcontainer.py              |  9 ++-----
 sdk/cwl/tests/test_container.py                  | 30 ++++++++++++------------
 sdk/go/arvados/config.go                         |  1 -
 services/api/app/models/container.rb             |  6 +++--
 services/api/test/unit/container_request_test.rb |  9 ++++---
 7 files changed, 31 insertions(+), 37 deletions(-)

       via  42dd3d670f9e9030145ee73e97d3ed2ba040bca5 (commit)
       via  745a7e253326a2b8e292dbd928c76320ce320216 (commit)
      from  0531cc9f35e78a7be1eec7eb96fb6cc668ebcefa (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 42dd3d670f9e9030145ee73e97d3ed2ba040bca5
Author: Tom Clegg <tom at curii.com>
Date:   Tue Dec 13 16:38:54 2022 -0500

    19847: Let server apply default disk cache size.
    
    Use disk cache when DefaultKeepCacheRAM==0 (instead of
    DefaultKeepCacheDisk>0).
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py
index c6030d723..a15c085da 100644
--- a/sdk/cwl/arvados_cwl/arvcontainer.py
+++ b/sdk/cwl/arvados_cwl/arvcontainer.py
@@ -264,7 +264,7 @@ class ArvadosContainer(JobBase):
         if api_req:
             runtime_constraints["API"] = True
 
-        use_disk_cache = (self.arvrunner.api.config()["Containers"].get("DefaultKeepCacheDisk", 0) > 0)
+        use_disk_cache = (self.arvrunner.api.config()["Containers"].get("DefaultKeepCacheRAM", 0) == 0)
 
         keep_cache_type_req, _ = self.get_requirement("http://arvados.org/cwl#KeepCacheTypeRequirement")
         if keep_cache_type_req:
@@ -276,7 +276,7 @@ class ArvadosContainer(JobBase):
         if runtime_req:
             if "keep_cache" in runtime_req:
                 if use_disk_cache:
-                    # If DefaultKeepCacheDisk is non-zero it means we should use disk cache.
+                    # If DefaultKeepCacheRAM is zero it means we should use disk cache.
                     runtime_constraints["keep_cache_disk"] = math.ceil(runtime_req["keep_cache"] * 2**20)
                 else:
                     runtime_constraints["keep_cache_ram"] = math.ceil(runtime_req["keep_cache"] * 2**20)
@@ -290,11 +290,6 @@ class ArvadosContainer(JobBase):
                         "writable": True
                     }
 
-        if use_disk_cache and "keep_cache_disk" not in runtime_constraints:
-            # Cache size wasn't explicitly set so set the default to
-            # 2 GB <= the size of the RAM request <= 32 GiB
-            runtime_constraints["keep_cache_disk"] = min(max(2*1024*1024*1024, runtime_constraints["ram"]), 32*1024*1024*1024)
-
         partition_req, _ = self.get_requirement("http://arvados.org/cwl#PartitionRequirement")
         if partition_req:
             scheduling_parameters["partitions"] = aslist(partition_req["partition"])
diff --git a/sdk/cwl/tests/test_container.py b/sdk/cwl/tests/test_container.py
index b8bc70782..bad4d4408 100644
--- a/sdk/cwl/tests/test_container.py
+++ b/sdk/cwl/tests/test_container.py
@@ -135,7 +135,7 @@ class TestContainer(unittest.TestCase):
             runner.intermediate_output_ttl = 0
             runner.secret_store = cwltool.secrets.SecretStore()
             runner.api._rootDesc = {"revision": "20210628"}
-            runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+            runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
             keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
             runner.api.collections().get().execute.return_value = {
@@ -202,7 +202,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 3600
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -300,7 +300,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -432,7 +432,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -666,7 +666,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -791,7 +791,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -890,7 +890,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -936,7 +936,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -1012,7 +1012,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -1108,7 +1108,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
         runner.api.collections().get().execute.return_value = {
@@ -1213,7 +1213,7 @@ class TestContainer(unittest.TestCase):
         runner.intermediate_output_ttl = 0
         runner.secret_store = cwltool.secrets.SecretStore()
         runner.api._rootDesc = {"revision": "20210628"}
-        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz4", {"dockerhash": "456"}),
                                    ("zzzzz-4zz18-zzzzzzzzzzzzzz3", {"dockerhash": "123"})]
@@ -1305,7 +1305,7 @@ class TestContainer(unittest.TestCase):
                 runner.intermediate_output_ttl = 0
                 runner.secret_store = cwltool.secrets.SecretStore()
                 runner.api._rootDesc = {"revision": "20210628"}
-                runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+                runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
                 keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
                 runner.api.collections().get().execute.return_value = {
@@ -1398,7 +1398,7 @@ class TestContainer(unittest.TestCase):
             runner.intermediate_output_ttl = 0
             runner.secret_store = cwltool.secrets.SecretStore()
             runner.api._rootDesc = {"revision": rev}
-            runner.api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+            runner.api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
             keepdocker.return_value = [("zzzzz-4zz18-zzzzzzzzzzzzzz3", "")]
             runner.api.collections().get().execute.return_value = {
@@ -1486,7 +1486,7 @@ class TestWorkflow(unittest.TestCase):
 
         api = mock.MagicMock()
         api._rootDesc = get_rootDesc()
-        api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         runner = arvados_cwl.executor.ArvCwlExecutor(api)
         self.assertEqual(runner.work_api, 'containers')
@@ -1619,7 +1619,7 @@ class TestWorkflow(unittest.TestCase):
 
         api = mock.MagicMock()
         api._rootDesc = get_rootDesc()
-        api.config.return_value = {"Containers": {"DefaultKeepCacheDisk": 0}}
+        api.config.return_value = {"Containers": {"DefaultKeepCacheRAM": 256<<20}}
 
         runner = arvados_cwl.executor.ArvCwlExecutor(api)
         self.assertEqual(runner.work_api, 'containers')

commit 745a7e253326a2b8e292dbd928c76320ce320216
Author: Tom Clegg <tom at curii.com>
Date:   Tue Dec 13 16:31:31 2022 -0500

    19847: Default to disk cache size = RAM size for all containers.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index f7c2beca3..e57bb7fb9 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -967,14 +967,14 @@ Clusters:
 
       # Default value for keep_cache_ram of a container's
       # runtime_constraints.  Note: this gets added to the RAM request
-      # used to allocate a VM or submit an HPC job
+      # used to allocate a VM or submit an HPC job.
+      #
+      # If this is zero, container requests that don't specify RAM or
+      # disk cache size will use a disk cache, sized to the
+      # container's RAM requirement (but with minimum 2 GiB and
+      # maximum 32 GiB).
       DefaultKeepCacheRAM: 0
 
-      # Default value for keep_cache_disk of a container's
-      # runtime_constraints.  Note: this gets added to the disk
-      # request used to allocate a VM or submit an HPC job
-      DefaultKeepCacheDisk: 8589934592
-
       # Number of times a container can be unlocked before being
       # automatically cancelled.
       MaxDispatchAttempts: 5
diff --git a/lib/config/export.go b/lib/config/export.go
index 814fc6cd9..6352406e9 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -121,7 +121,6 @@ var whitelist = map[string]bool{
 	"Containers.CrunchRunArgumentsList":        false,
 	"Containers.CrunchRunCommand":              false,
 	"Containers.DefaultKeepCacheRAM":           true,
-	"Containers.DefaultKeepCacheDisk":          true,
 	"Containers.DispatchPrivateKey":            false,
 	"Containers.JobsAPI":                       true,
 	"Containers.JobsAPI.Enable":                true,
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index bc6aab298..2871356e9 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -447,7 +447,6 @@ type ContainersConfig struct {
 	CrunchRunCommand              string
 	CrunchRunArgumentsList        []string
 	DefaultKeepCacheRAM           ByteSize
-	DefaultKeepCacheDisk          ByteSize
 	DispatchPrivateKey            string
 	LogReuseDecisions             bool
 	MaxComputeVMs                 int
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 4b02ad52e..21e8fcf50 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -228,8 +228,10 @@ class Container < ArvadosModel
       rc['keep_cache_ram'] = Rails.configuration.Containers.DefaultKeepCacheRAM
     end
     if rc['keep_cache_disk'] == 0 and rc['keep_cache_ram'] == 0
-      # Only set if keep_cache_ram isn't set.
-      rc['keep_cache_disk'] = Rails.configuration.Containers.DefaultKeepCacheDisk
+      # If neither ram nor disk cache was specified and
+      # DefaultKeepCacheRAM==0, default to disk cache with size equal
+      # to RAM constraint (but at least 2 GiB and at most 32 GiB).
+      rc['keep_cache_disk'] = [[rc['ram'] || 0, 2 << 30].max, 32 << 30].min
     end
     rc
   end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index efc61eee8..c7b5da5dd 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -107,7 +107,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     {"runtime_constraints" => {"vcpus" => 1}},
     {"runtime_constraints" => {"vcpus" => 1, "ram" => nil}},
     {"runtime_constraints" => {"vcpus" => 0, "ram" => 123}},
-    {"runtime_constraints" => {"vcpus" => "1", "ram" => "123"}},
+    {"runtime_constraints" => {"vcpus" => "1", "ram" => -1}},
     {"mounts" => {"FOO" => "BAR"}},
     {"mounts" => {"FOO" => {}}},
     {"mounts" => {"FOO" => {"kind" => "tmp", "capacity" => 42.222}}},
@@ -164,7 +164,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
   test "Container request commit" do
     set_user_from_auth :active
-    cr = create_minimal_req!(runtime_constraints: {"vcpus" => 2, "ram" => 30})
+    cr = create_minimal_req!(runtime_constraints: {"vcpus" => 2, "ram" => 300000000})
 
     assert_nil cr.container_uuid
 
@@ -175,10 +175,9 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
     cr.reload
 
-    assert ({"vcpus" => 2, "ram" => 30}.to_a - cr.runtime_constraints.to_a).empty?
+    assert_empty({"vcpus" => 2, "ram" => 300000000}.to_a - cr.runtime_constraints.to_a)
 
     assert_equal 0, Rails.configuration.Containers.DefaultKeepCacheRAM
-    assert_equal 8589934592, Rails.configuration.Containers.DefaultKeepCacheDisk
 
     assert_not_nil cr.container_uuid
     c = Container.find_by_uuid cr.container_uuid
@@ -189,7 +188,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
     assert_equal({}, c.environment)
     assert_equal({"/out" => {"kind"=>"tmp", "capacity"=>1000000}}, c.mounts)
     assert_equal "/out", c.output_path
-    assert ({"keep_cache_disk"=>8589934592, "keep_cache_ram"=>0, "vcpus" => 2, "ram" => 30}.to_a - c.runtime_constraints.to_a).empty?
+    assert ({"keep_cache_disk" => 2<<30, "keep_cache_ram" => 0, "vcpus" => 2, "ram" => 300000000}.to_a - c.runtime_constraints.to_a).empty?
     assert_operator 0, :<, c.priority
 
     assert_raises(ActiveRecord::RecordInvalid) do

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list