19847: Default to disk cache size = RAM size for all containers.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index f7c2beca3372f294bb16762d6f5366e7e989a84c..e57bb7fb9474efd2f1603feb381d024429c94a9f 100644 (file)
--- 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 814fc6cd9b9dfc6ab0fbea0d9e29715236a906bd..6352406e90e95dc255d9eb43ff1ca13f0e721fd1 100644 (file)
--- 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 bc6aab298fdcab19991528c1949b9d7bbac5efaf..2871356e9827059352026432082c5fcdee2f3fce 100644 (file)
--- 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 4b02ad52e56dc55925d9418e528434917897379e..21e8fcf508c4ee0951d4f43abe2c71702e40e62f 100644 (file)
--- 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 efc61eee8c80fcfa509478ebc0ec51e947e3cccb..c7b5da5ddc2f4f2fe93fb02a5a0e4030b028d742 100644 (file)
--- 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