From d3eb7fa3f6d703f8d648f3ce795ab8fdce898fd6 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Thu, 31 Oct 2024 16:19:21 -0400 Subject: [PATCH 01/40] wip --- test/Table_Tests/src/IO/Fetch_Spec.enso | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 125d84123f08..a717d28c627b 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -14,6 +14,7 @@ from Standard.Table import all import Standard.Table.Errors.Invalid_JSON_Format from Standard.Test import all +import Standard.Test.Test_Environment import enso_dev.Base_Tests.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup from enso_dev.Base_Tests.Network.Http.Http_Test_Setup import base_url_with_slash, pending_has_url @@ -430,8 +431,12 @@ add_specs suite_builder = HTTP.fetch headers=headers2 uri get_num_response_cache_entries . should_equal 2 - group_builder.specify "Should not be able to set the cache limits higher than the real limits" pending=pending_has_url <| Test.with_retries <| + group_builder.specify "Should not be able to set the cache limits higher than the real limits" <| Test.expect_panic IllegalArgumentException <| EnsoHTTPResponseCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument Test.expect_panic IllegalArgumentException <| EnsoHTTPResponseCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument + + group_builder.specify "Should be able to set the max file size via environment variable" <| + group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| + group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| From 8064cfbe8699e128b400e5ca0a3e29dc12333966 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 1 Nov 2024 16:36:47 -0400 Subject: [PATCH 02/40] make EHRC static --- .../Base/0.0.0-dev/src/Network/HTTP.enso | 3 +- .../enso_cloud/EnsoHTTPResponseCache.java | 30 +++++++++++-------- .../base/enso_cloud/EnsoSecretHelper.java | 7 ++++- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso index 296948c0d239..a34d38561329 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso @@ -49,7 +49,6 @@ polyglot java import java.net.http.HttpRequest.Builder polyglot java import java.net.InetSocketAddress polyglot java import java.net.ProxySelector polyglot java import javax.net.ssl.SSLContext -polyglot java import org.enso.base.enso_cloud.EnsoHTTPResponseCache polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper polyglot java import org.enso.base.file_system.File_Utils polyglot java import org.enso.base.net.http.MultipartBodyBuilder @@ -184,7 +183,7 @@ type HTTP HTTP.clear_response_cache clear_response_cache : Nothing - clear_response_cache -> Nothing = EnsoHTTPResponseCache.clear + clear_response_cache -> Nothing = EnsoSecretHelper.clearCache ## PRIVATE ADVANCED diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index 12187214b17f..22c03cf8a459 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -21,15 +21,19 @@ * is no cache hit, the InputStream is connected directly to the remote server. */ public class EnsoHTTPResponseCache { + // Public for testing. + public EnsoHTTPResponseCache() { + } + // 1 year. - private static final int DEFAULT_TTL_SECONDS = 31536000; - private static final long MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; - private static final long MAX_TOTAL_CACHE_SIZE = 20L * 1024 * 1024 * 1024; + private final int DEFAULT_TTL_SECONDS = 31536000; + private final long MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; + private final long MAX_TOTAL_CACHE_SIZE = 20L * 1024 * 1024 * 1024; - private static final LRUCache lruCache = + private final LRUCache lruCache = new LRUCache<>(MAX_FILE_SIZE, MAX_TOTAL_CACHE_SIZE); - public static EnsoHttpResponse makeRequest(RequestMaker requestMaker) + public EnsoHttpResponse makeRequest(RequestMaker requestMaker) throws IOException, InterruptedException, ResponseTooLargeException { var itemBuilder = new ItemBuilder(requestMaker); @@ -39,7 +43,7 @@ public static EnsoHttpResponse makeRequest(RequestMaker requestMaker) cacheResult.inputStream(), cacheResult.metadata()); } - public static class ItemBuilder implements LRUCache.ItemBuilder { + public class ItemBuilder implements LRUCache.ItemBuilder { private final RequestMaker requestMaker; ItemBuilder(RequestMaker requestMaker) { @@ -74,7 +78,7 @@ public LRUCache.Item buildItem() throws IOException, InterruptedExcept } /** Get the size of the response data, if available. */ - private static Optional getResponseDataSize(HttpHeaders headers) { + private Optional getResponseDataSize(HttpHeaders headers) { return headers.firstValue("content-length").map(Long::parseLong); } @@ -94,7 +98,7 @@ private static Optional getResponseDataSize(HttpHeaders headers) { *

If 'max-age' and 'Age' are both present, we set TTL = max-age - Age. If only 'max-age' is * present, we set TTL = max-age. If neither are present, we use a default. */ - private static int calculateTTL(HttpHeaders headers) { + private int calculateTTL(HttpHeaders headers) { Integer maxAge = getMaxAge(headers); if (maxAge == null) { return DEFAULT_TTL_SECONDS; @@ -104,7 +108,7 @@ private static int calculateTTL(HttpHeaders headers) { } } - private static Integer getMaxAge(HttpHeaders headers) { + private Integer getMaxAge(HttpHeaders headers) { var cacheControlMaybe = headers.firstValue("cache-control"); Integer maxAge = null; if (cacheControlMaybe.isPresent()) { @@ -123,20 +127,20 @@ private static Integer getMaxAge(HttpHeaders headers) { return maxAge; } - public static void clear() { + public void clear() { lruCache.clear(); } - public static int getNumEntries() { + public int getNumEntries() { return lruCache.getNumEntries(); } - public static List getFileSizesTestOnly() { + public List getFileSizesTestOnly() { return lruCache.getFileSizesTestOnly(); } /** Return a set of parameters that can be used to modify settings for testing purposes. */ - public static LRUCache.CacheTestParameters getCacheTestParameters() { + public LRUCache.CacheTestParameters getCacheTestParameters() { return lruCache.getCacheTestParameters(); } diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 3e4717fee5ab..2fdf288b61c0 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -22,6 +22,7 @@ /** Makes HTTP requests with secrets in either header or query string. */ public final class EnsoSecretHelper extends SecretValueResolver { + private static final EnsoHTTPResponseCache cache = new EnsoHTTPResponseCache(); /** Gets a JDBC connection resolving EnsoKeyValuePair into the properties. */ public static Connection getJDBCConnection( @@ -87,7 +88,7 @@ public static EnsoHttpResponse makeRequest( if (!useCache) { return requestMaker.makeRequest(); } else { - return EnsoHTTPResponseCache.makeRequest(requestMaker); + return cache.makeRequest(requestMaker); } } @@ -175,6 +176,10 @@ public EnsoHttpResponse reconstructResponseFromCachedStream( } } + public static final clearCache() { + cache.clear(); + } + private static final Comparator> headerNameComparator = Comparator.comparing((Pair pair) -> pair.getLeft()) .thenComparing(Comparator.comparing(pair -> pair.getRight())); From 12c3733e7deedf87c1dc1da99c89467756e58110 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 4 Nov 2024 14:20:17 -0500 Subject: [PATCH 03/40] EHRC singleton --- .../Base/0.0.0-dev/src/Network/HTTP.enso | 2 +- .../base/enso_cloud/EnsoSecretHelper.java | 4 +- test/Table_Tests/src/IO/Fetch_Spec.enso | 37 ++++++++++--------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso index a34d38561329..45634768d6c3 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso @@ -183,7 +183,7 @@ type HTTP HTTP.clear_response_cache clear_response_cache : Nothing - clear_response_cache -> Nothing = EnsoSecretHelper.clearCache + clear_response_cache -> Nothing = EnsoSecretHelper.getCache.clear ## PRIVATE ADVANCED diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 2fdf288b61c0..6516b2797a7b 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -176,8 +176,8 @@ public EnsoHttpResponse reconstructResponseFromCachedStream( } } - public static final clearCache() { - cache.clear(); + public static EnsoHTTPResponseCache getCache() { + return cache; } private static final Comparator> headerNameComparator = diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index a717d28c627b..997d2e22db57 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -22,7 +22,7 @@ from enso_dev.Base_Tests.Network.Http.Http_Test_Setup import base_url_with_slash import project.Util polyglot java import java.lang.IllegalArgumentException -polyglot java import org.enso.base.enso_cloud.EnsoHTTPResponseCache +polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper main filter=Nothing = suite = Test.build suite_builder-> @@ -108,7 +108,7 @@ add_specs suite_builder = suite_builder.group "Response caching" pending=pending_has_url group_builder-> get_num_response_cache_entries = - EnsoHTTPResponseCache.getNumEntries + EnsoSecretHelper.getCache.getNumEntries with_counts ~action = before_count = get_num_response_cache_entries action @@ -116,8 +116,8 @@ add_specs suite_builder = [before_count, after_count] reset_size_limits = - EnsoHTTPResponseCache.getCacheTestParameters.clearMaxFileSizeOverrideTestOnly - EnsoHTTPResponseCache.getCacheTestParameters.clearMaxTotalCacheSizeOverrideTestOnly + EnsoSecretHelper.getCache.getCacheTestParameters.clearMaxFileSizeOverrideTestOnly + EnsoSecretHelper.getCache.getCacheTestParameters.clearMaxTotalCacheSizeOverrideTestOnly expect_counts expected_counts ~action = counts = with_counts action @@ -125,7 +125,7 @@ add_specs suite_builder = get_cache_file_sizes : Vector Integer get_cache_file_sizes -> Vector Integer = - Vector.from_polyglot_array EnsoHTTPResponseCache.getFileSizesTestOnly . sort Sort_Direction.Ascending + Vector.from_polyglot_array EnsoSecretHelper.getCache.getFileSizesTestOnly . sort Sort_Direction.Ascending url0 = base_url_with_slash+'test_download?max-age=16&length=10' url1 = base_url_with_slash+'test_download?max-age=16&length=20' @@ -220,8 +220,8 @@ add_specs suite_builder = group_builder.specify "Data.download is not affected by caching limits" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" Panic.with_finalizer reset_size_limits <| - EnsoHTTPResponseCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 120 - EnsoHTTPResponseCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 120 + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 Data.download base_url_with_slash+"test_download?length=200" target_file target_file.read.length . should_equal 200 target_file.delete_if_exists @@ -277,12 +277,12 @@ add_specs suite_builder = trigger_uri_serial.modify (_ + 1) base_url_with_slash+'test_download?max-age=10000&length=50&abc='+serial.to_text set_time_and_get_count advance_secs = - EnsoHTTPResponseCache.getCacheTestParameters.setNowOverrideTestOnly (fake_now + (Duration.new seconds=advance_secs)) + EnsoSecretHelper.getCache.getCacheTestParameters.setNowOverrideTestOnly (fake_now + (Duration.new seconds=advance_secs)) trigger_uri = make_trigger_uri Data.fetch trigger_uri get_num_response_cache_entries fake_time_resetter = - EnsoHTTPResponseCache.getCacheTestParameters.clearNowOverrideTestOnly + EnsoSecretHelper.getCache.getCacheTestParameters.clearNowOverrideTestOnly group_builder.specify "The cache should expire stale entries" pending=pending_has_url <| Test.with_retries <| HTTP.clear_response_cache @@ -333,7 +333,7 @@ add_specs suite_builder = group_builder.specify "Will remove old cache files to keep the total cache size under the total cache size limit" pending=pending_has_url <| Test.with_retries <| Panic.with_finalizer reset_size_limits <| reset_size_limits - EnsoHTTPResponseCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 download 30 download 50 @@ -349,7 +349,7 @@ add_specs suite_builder = group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| Panic.with_finalizer reset_size_limits <| reset_size_limits - EnsoHTTPResponseCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 download 30 download 50 @@ -366,7 +366,7 @@ add_specs suite_builder = group_builder.specify "Will not cache a file with a content length greater than the single file limit" pending=pending_has_url <| Test.with_retries <| Panic.with_finalizer reset_size_limits <| reset_size_limits - EnsoHTTPResponseCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 download 110 . should_fail_with (Response_Too_Large.Error 100) @@ -374,7 +374,7 @@ add_specs suite_builder = HTTP.clear_response_cache Panic.with_finalizer reset_size_limits <| reset_size_limits - EnsoHTTPResponseCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 url = base_url_with_slash+'test_download?omit-content-length=1&length=110' Data.fetch url . should_fail_with (Response_Too_Large.Error 100) @@ -433,10 +433,11 @@ add_specs suite_builder = group_builder.specify "Should not be able to set the cache limits higher than the real limits" <| Test.expect_panic IllegalArgumentException <| - EnsoHTTPResponseCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument Test.expect_panic IllegalArgumentException <| - EnsoHTTPResponseCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument - group_builder.specify "Should be able to set the max file size via environment variable" <| - group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| - group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| + ## + group_builder.specify "Should be able to set the max file size via environment variable" <| + group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| + group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| From 8f7c0abe01e7fc95e28bc9ac5fdba28c1f776b15 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 4 Nov 2024 16:16:06 -0500 Subject: [PATCH 04/40] wip --- .../java/org/enso/base/cache/LRUCache.java | 80 +++++++++++++++++-- .../org/enso/base/cache/TotalCacheLimit.java | 25 ++++++ .../enso_cloud/EnsoHTTPResponseCache.java | 10 ++- test/Table_Tests/src/IO/Fetch_Spec.enso | 24 +++++- 4 files changed, 125 insertions(+), 14 deletions(-) create mode 100644 std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index df1329da6a9c..99a6284c1d2c 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -33,17 +33,25 @@ public class LRUCache { private static final Logger logger = Logger.getLogger(LRUCache.class.getName()); + private final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; + private final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; + private final long MIN_TOTAL_CACHE_SIZE_FREE_SPACE = 20 * 1024 * 1024 * 1024; + private final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE = 100 * 1024 * 1024 * 1024; + private final long maxFileSize; - private final long maxTotalCacheSize; + private final TotalCacheLimit.Limit totalCacheLimit; private final CacheTestParameters cacheTestParameters = new CacheTestParameters(); private final Map> cache = new HashMap<>(); private final Map lastUsed = new HashMap<>(); - public LRUCache(long maxFileSize, long maxTotalCacheSize) { + // TODO: use the project root dir. + private final File rootDir = new File("/"); + + public LRUCache(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) { this.maxFileSize = maxFileSize; - this.maxTotalCacheSize = maxTotalCacheSize; + this.totalCacheLimit = totalCacheLimit; } public CacheResult getResult(ItemBuilder itemBuilder) @@ -230,12 +238,35 @@ private long getTotalCacheSize() { return cache.values().stream().collect(Collectors.summingLong(e -> e.size())); } - private long getMaxFileSize() { + // Public for testing. + public long getMaxFileSize() { return cacheTestParameters.getMaxFileSizeOverrideTestOnly().orElse(maxFileSize); } - private long getMaxTotalCacheSize() { - return cacheTestParameters.getMaxTotalCacheSizeOverrideTestOnly().orElse(maxTotalCacheSize); + public TotalCacheLimit.Limit getTotalCacheLimit() { + return totalCacheLimit; + } + + // Public for testing. + public long getMaxTotalCacheSize() { + return cacheTestParameters.getMaxTotalCacheSizeOverrideTestOnly().orElse(calculateTotalCacheSize()); + } + + private long calculateTotalCacheSize() { + return switch (totalCacheLimit) { + case TotalCacheLimit.Megs megs -> (long) (megs.megs() * 1024 * 1024); + case TotalCacheLimit.Percentage percentage -> { + long usableSpace = getUsableDiskSpace(); + long totalCacheSize = (long) (percentage.percentage() * usableSpace): + return + Long.max(MIN_TOTAL_CACHE_SIZE_FREE_SPACE, + Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE, totalCacheSize)); + }; + }; + } + + private long getUsableDiskSpace() { + return cacheTestParameters.getUsableDiskSpaceOverrideTestOnly().orElse(rootDir.getUsableDiskSpace()); } public int getNumEntries() { @@ -304,6 +335,8 @@ public class CacheTestParameters { private Optional maxTotalCacheSizeOverrideTestOnly = Optional.empty(); + private Optional usableDiskSpaceOverrideTestOnly = Optional.empty(); + public Optional getNowOverrideTestOnly() { return nowOverrideTestOnly; } @@ -349,5 +382,40 @@ public void setMaxTotalCacheSizeOverrideTestOnly(long maxTotalCacheSizeOverrideT public void clearMaxTotalCacheSizeOverrideTestOnly() { maxTotalCacheSizeOverrideTestOnly = Optional.empty(); } + + public Optional getUsableDiskSpaceOverrideTestOnly() { + return usableDiskSpaceOverrideTestOnly; + } + + public void setUsableDiskSpaceOverrideTestOnly(long usableDiskSpaceOverrideTestOnly_) { + usableDiskSpaceOverrideTestOnly = Optional.of(usableDiskSpaceOverrideTestOnly_); + } + + public void clearUsableDiskSpaceOverrideTestOnly() { + usableDiskSpaceOverrideTestOnly = Optional.empty(); + } + } + + private static LRUCache<> create() { + return new LRUCache<>(calcMaxFileSize(), calcTotalCacheLimit()); + } + + private long calcMaxFileSize() { + String maxFileSizeMegsVar = System.getenv("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); + if (maxFileSizeMegsVar != null) { + double maxFileSizeMegs = Double.parseDouble(maxFileSizeMegsVar); + return (long) (maxFileSizeMegs * 1024 * 1024); + } else { + return DEFAULT_MAX_FILE_SIZE; + } + } + + private TotalCacheLimit.Limit calcTotalCacheLimit() { + String limitVar = System.getenv("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); + if (limitVar != null) { + return TotalCacheLimit.parse(limitVar); + } else { + return new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + } } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java new file mode 100644 index 000000000000..4e7d481c17ac --- /dev/null +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -0,0 +1,25 @@ +package org.enso.base.cache; + +import java.text.DecimalFormat; +import java.text.ParsePosition; + +public class TotalCacheLimit { + public static Limit parse(String limitString) throws NumberFormatException { + Number percentage = tryPercentage(limitString): + if (percentage != null) { + double percentage = percentage.doubleValue(); + if (percentage < 0.0 + return new Percentage(); + } + return new Megs(Double.parseDouble(limitString)); + } + + public sealed interface Limit permits Megs, Percentage; + + // Specify the limit in megs. + public record Megs(double megs) implements TotalCacheLimit () + + // Specify the limit as a percentage of total free, usable disk space. + public record Percentage(double percentage) implements TotalCacheLimit {} + } +} diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index 22c03cf8a459..45cd256694b3 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Optional; import org.enso.base.cache.LRUCache; +import org.enso.base.cache.TotalCacheLimit; import org.enso.base.cache.ResponseTooLargeException; /** @@ -27,11 +28,8 @@ public EnsoHTTPResponseCache() { // 1 year. private final int DEFAULT_TTL_SECONDS = 31536000; - private final long MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; - private final long MAX_TOTAL_CACHE_SIZE = 20L * 1024 * 1024 * 1024; - private final LRUCache lruCache = - new LRUCache<>(MAX_FILE_SIZE, MAX_TOTAL_CACHE_SIZE); + private final LRUCache lruCache = LRUCache.create(); public EnsoHttpResponse makeRequest(RequestMaker requestMaker) throws IOException, InterruptedException, ResponseTooLargeException { @@ -144,6 +142,10 @@ public LRUCache.CacheTestParameters getCacheTestParameters() { return lruCache.getCacheTestParameters(); } + public LRUCache getLRUCache() { + return lruCache; + } + public interface RequestMaker { /** Executes the HTTP request and returns the response. */ EnsoHttpResponse makeRequest() throws IOException, InterruptedException; diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 997d2e22db57..fef1d1b250b8 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -22,6 +22,7 @@ from enso_dev.Base_Tests.Network.Http.Http_Test_Setup import base_url_with_slash import project.Util polyglot java import java.lang.IllegalArgumentException +polyglot java import org.enso.base.cache.LRUCache polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper main filter=Nothing = @@ -118,6 +119,8 @@ add_specs suite_builder = reset_size_limits = EnsoSecretHelper.getCache.getCacheTestParameters.clearMaxFileSizeOverrideTestOnly EnsoSecretHelper.getCache.getCacheTestParameters.clearMaxTotalCacheSizeOverrideTestOnly + reset_disk_space = + EnsoSecretHelper.getCache.getCacheTestParameters.clearUsableDiskSpaceOverrideTestOnly expect_counts expected_counts ~action = counts = with_counts action @@ -437,7 +440,20 @@ add_specs suite_builder = Test.expect_panic IllegalArgumentException <| EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument - ## - group_builder.specify "Should be able to set the max file size via environment variable" <| - group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| - group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| + group_builder.specify "Cache limits should have defaults" <| + lru_cache = EnsoSecretHelper.getCache.getLRUCache + lru_cache.getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) + lru_cache.getTotalCacheLimit.percentage . should_equal 0.2 + + group_builder.specify "Should be able to set the max file size via environment variable" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" 8 <| + lru_cache = LRUCache.create + lru_cache.getMaxFileSize . should_equal (8 * 1024 * 1024) + + group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| + group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| + # check the size and the percentage using fake disk space + Panic.with_finalizer reset_disk_space <| + EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 100 + group_builder.specify "Toital cache size should be constrainted to the allowed range" <| + group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| From 54cad982d853335acb8970ea5e399fb66ced6798 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 4 Nov 2024 17:01:33 -0500 Subject: [PATCH 05/40] wip --- test/Table_Tests/src/IO/Fetch_Spec.enso | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index fef1d1b250b8..af714caab918 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -446,14 +446,21 @@ add_specs suite_builder = lru_cache.getTotalCacheLimit.percentage . should_equal 0.2 group_builder.specify "Should be able to set the max file size via environment variable" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" 8 <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| lru_cache = LRUCache.create lru_cache.getMaxFileSize . should_equal (8 * 1024 * 1024) group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| + lru_cache = LRUCache.create + lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) + group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| - # check the size and the percentage using fake disk space - Panic.with_finalizer reset_disk_space <| - EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 100 + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| + Panic.with_finalizer reset_disk_space <| + EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (300 * 1024 * 1024 * 1024) + lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) + lru.getTotalCacheLimit.percentage . should_equal 0.1 + group_builder.specify "Toital cache size should be constrainted to the allowed range" <| group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| From c071438fbabafa62e386c47fd0c186156e442c15 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 11:17:11 -0500 Subject: [PATCH 06/40] tests pass --- .../java/org/enso/base/cache/LRUCache.java | 36 +++++++++---------- .../org/enso/base/cache/TotalCacheLimit.java | 23 +++++++----- .../enso_cloud/EnsoHTTPResponseCache.java | 2 +- test/Table_Tests/src/IO/Fetch_Spec.enso | 28 +++++++++++---- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 99a6284c1d2c..9ae639a37ae1 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -19,6 +19,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import org.enso.base.Environment_Utils; import org.enso.base.Stream_Utils; /** @@ -33,10 +34,9 @@ public class LRUCache { private static final Logger logger = Logger.getLogger(LRUCache.class.getName()); - private final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; - private final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; - private final long MIN_TOTAL_CACHE_SIZE_FREE_SPACE = 20 * 1024 * 1024 * 1024; - private final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE = 100 * 1024 * 1024 * 1024; + private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; + private static final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; + private static final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE = 100L * 1024 * 1024 * 1024; private final long maxFileSize; private final TotalCacheLimit.Limit totalCacheLimit; @@ -49,6 +49,11 @@ public class LRUCache { // TODO: use the project root dir. private final File rootDir = new File("/"); + + public LRUCache() { + this(calcMaxFileSize(), calcTotalCacheLimit()); + } + public LRUCache(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) { this.maxFileSize = maxFileSize; this.totalCacheLimit = totalCacheLimit; @@ -257,16 +262,14 @@ private long calculateTotalCacheSize() { case TotalCacheLimit.Megs megs -> (long) (megs.megs() * 1024 * 1024); case TotalCacheLimit.Percentage percentage -> { long usableSpace = getUsableDiskSpace(); - long totalCacheSize = (long) (percentage.percentage() * usableSpace): - return - Long.max(MIN_TOTAL_CACHE_SIZE_FREE_SPACE, - Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE, totalCacheSize)); - }; + long totalCacheSize = (long) (percentage.percentage() * usableSpace); + yield Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE, totalCacheSize); + } }; } private long getUsableDiskSpace() { - return cacheTestParameters.getUsableDiskSpaceOverrideTestOnly().orElse(rootDir.getUsableDiskSpace()); + return cacheTestParameters.getUsableDiskSpaceOverrideTestOnly().orElse(rootDir.getUsableSpace()); } public int getNumEntries() { @@ -371,6 +374,7 @@ public Optional getMaxTotalCacheSizeOverrideTestOnly() { } public void setMaxTotalCacheSizeOverrideTestOnly(long maxTotalCacheSizeOverrideTestOnly_) { + long maxTotalCacheSize = getMaxTotalCacheSize(); if (maxTotalCacheSizeOverrideTestOnly_ > maxTotalCacheSize) { throw new IllegalArgumentException( "Cannot set the (test-only) total cache size to more than the allowed limit of " @@ -396,12 +400,8 @@ public void clearUsableDiskSpaceOverrideTestOnly() { } } - private static LRUCache<> create() { - return new LRUCache<>(calcMaxFileSize(), calcTotalCacheLimit()); - } - - private long calcMaxFileSize() { - String maxFileSizeMegsVar = System.getenv("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); + private static long calcMaxFileSize() { + String maxFileSizeMegsVar = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); if (maxFileSizeMegsVar != null) { double maxFileSizeMegs = Double.parseDouble(maxFileSizeMegsVar); return (long) (maxFileSizeMegs * 1024 * 1024); @@ -410,8 +410,8 @@ private long calcMaxFileSize() { } } - private TotalCacheLimit.Limit calcTotalCacheLimit() { - String limitVar = System.getenv("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); + private static TotalCacheLimit.Limit calcTotalCacheLimit() { + String limitVar = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); if (limitVar != null) { return TotalCacheLimit.parse(limitVar); } else { diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java index 4e7d481c17ac..20a4b6cd0929 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -5,21 +5,28 @@ public class TotalCacheLimit { public static Limit parse(String limitString) throws NumberFormatException { - Number percentage = tryPercentage(limitString): - if (percentage != null) { - double percentage = percentage.doubleValue(); - if (percentage < 0.0 - return new Percentage(); + Number percentageNumber = tryPercentage(limitString); + if (percentageNumber != null) { + double percentage = percentageNumber.doubleValue(); + if (percentage < 0.0 || percentage > 1.0) { + throw new IllegalArgumentException("LURCache free disk space percentage must be in the range 0..100% (inclusive): was " + limitString); + } + return new Percentage(percentage); } return new Megs(Double.parseDouble(limitString)); } - public sealed interface Limit permits Megs, Percentage; + public sealed interface Limit permits Megs, Percentage {} // Specify the limit in megs. - public record Megs(double megs) implements TotalCacheLimit () + public record Megs(double megs) implements Limit {} // Specify the limit as a percentage of total free, usable disk space. - public record Percentage(double percentage) implements TotalCacheLimit {} + public record Percentage(double percentage) implements Limit {} + + private static Number tryPercentage(String limitString) { + DecimalFormat df = new DecimalFormat("0%"); + ParsePosition pp = new ParsePosition(0); + return df.parse(limitString, pp); } } diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index 45cd256694b3..90f407d7f0cb 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -29,7 +29,7 @@ public EnsoHTTPResponseCache() { // 1 year. private final int DEFAULT_TTL_SECONDS = 31536000; - private final LRUCache lruCache = LRUCache.create(); + private final LRUCache lruCache = new LRUCache<>(); public EnsoHttpResponse makeRequest(RequestMaker requestMaker) throws IOException, InterruptedException, ResponseTooLargeException { diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index af714caab918..ffef554d1aff 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -22,6 +22,7 @@ from enso_dev.Base_Tests.Network.Http.Http_Test_Setup import base_url_with_slash import project.Util polyglot java import java.lang.IllegalArgumentException +polyglot java import java.lang.NumberFormatException polyglot java import org.enso.base.cache.LRUCache polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper @@ -447,20 +448,35 @@ add_specs suite_builder = group_builder.specify "Should be able to set the max file size via environment variable" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - lru_cache = LRUCache.create + lru_cache = LRUCache.new lru_cache.getMaxFileSize . should_equal (8 * 1024 * 1024) group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| - lru_cache = LRUCache.create + lru_cache = LRUCache.new lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| Panic.with_finalizer reset_disk_space <| - EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (300 * 1024 * 1024 * 1024) - lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) - lru.getTotalCacheLimit.percentage . should_equal 0.1 + lru_cache = LRUCache.new + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (300 * 1024 * 1024 * 1024) + lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024 * 1024) + lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 + + group_builder.specify "Toital cache size should not go over the hard-coded maximum" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50%" <| + Panic.with_finalizer reset_disk_space <| + lru_cache = LRUCache.new + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (500 * 1024 * 1024 * 1024) + lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) - group_builder.specify "Toital cache size should be constrainted to the allowed range" <| group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| + Test.expect_panic NumberFormatException LRUCache.new + + group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| + Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| + Test.expect_panic IllegalArgumentException LRUCache.new From 232c88ba21f0bc81af204fa00d7ffaade11b0d8f Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 11:40:42 -0500 Subject: [PATCH 07/40] use enso project root --- .../Base/0.0.0-dev/src/Meta/Enso_Project.enso | 5 +++++ .../java/org/enso/base/CurrentEnsoProject.java | 15 +++++++++++---- .../main/java/org/enso/base/cache/LRUCache.java | 7 ++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Meta/Enso_Project.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Meta/Enso_Project.enso index 7a7af704033a..feccbb2e7476 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Meta/Enso_Project.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Meta/Enso_Project.enso @@ -91,6 +91,11 @@ type Project_Description namespace : Text namespace self = self.ns + ## PRIVATE + Returns the path of the project root. + root_path : Text + root_path self = self.root.path + ## ICON enso_icon Returns the Enso project description for the project that the engine was executed with, i.e., the project that contains the `main` method, or diff --git a/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java b/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java index 2928c693d58c..dccbdf27497e 100644 --- a/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java +++ b/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java @@ -7,26 +7,29 @@ public final class CurrentEnsoProject { private final String name; private final String namespace; + private final String rootPath; private static CurrentEnsoProject cached = null; private static boolean isCached = false; - private CurrentEnsoProject(String name, String namespace) { + private CurrentEnsoProject(String name, String namespace, String rootPath) { this.name = name; this.namespace = namespace; + this.rootPath = rootPath; } public static CurrentEnsoProject get() { if (!isCached) { Value ensoProject = EnsoMeta.callStaticModuleMethod("Standard.Base.Meta.Enso_Project", "enso_project"); - if (ensoProject.hasMember("name") && ensoProject.hasMember("namespace")) { + if (ensoProject.hasMember("name") && ensoProject.hasMember("namespace") && ensoProject.hasMember("root_path")) { Value namespace = ensoProject.invokeMember("namespace"); Value name = ensoProject.invokeMember("name"); - if (namespace == null || name == null) { + Value rootPath = ensoProject.invokeMember("root_path"); + if (namespace == null || name == null || rootPath == null) { cached = null; } else { - cached = new CurrentEnsoProject(name.asString(), namespace.asString()); + cached = new CurrentEnsoProject(name.asString(), namespace.asString(), rootPath.asString()); } } else { cached = null; @@ -46,6 +49,10 @@ public String getNamespace() { return namespace; } + public String getRootPath() { + return rootPath; + } + public String fullName() { return namespace + "." + name; } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 9ae639a37ae1..51999b08783e 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -19,6 +19,8 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; + +import org.enso.base.CurrentEnsoProject; import org.enso.base.Environment_Utils; import org.enso.base.Stream_Utils; @@ -47,8 +49,7 @@ public class LRUCache { private final Map lastUsed = new HashMap<>(); // TODO: use the project root dir. - private final File rootDir = new File("/"); - + private static final File rootPath = new File(CurrentEnsoProject.get().getRootPath()); public LRUCache() { this(calcMaxFileSize(), calcTotalCacheLimit()); @@ -269,7 +270,7 @@ private long calculateTotalCacheSize() { } private long getUsableDiskSpace() { - return cacheTestParameters.getUsableDiskSpaceOverrideTestOnly().orElse(rootDir.getUsableSpace()); + return cacheTestParameters.getUsableDiskSpaceOverrideTestOnly().orElse(rootPath.getUsableSpace()); } public int getNumEntries() { From c744a582339230c793f06d66f109bb814a078b03 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 11:52:09 -0500 Subject: [PATCH 08/40] docs, rename --- .../java/org/enso/base/cache/LRUCache.java | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 51999b08783e..49fa2aaccd4e 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -36,19 +36,32 @@ public class LRUCache { private static final Logger logger = Logger.getLogger(LRUCache.class.getName()); + /** Default value for the largest file size allowed. */ private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; + + /** Default value for the percentage of free disk space to use as a limit on the total cache size. */ private static final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; - private static final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE = 100L * 1024 * 1024 * 1024; + /** + * An upper limit on the total cache size. If the cache size limit specified + * by the other parameters goes over this value, then this value is used. + */ + private static final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND = 100L * 1024 * 1024 * 1024; + + /** + * Maximum size allowed for a single file. If a file larger than this is + * requested through this cache, a ResponseTooLargeException is thrown. + */ private final long maxFileSize; + /** Limits the total size of all files in the cache. */ private final TotalCacheLimit.Limit totalCacheLimit; + /** Used to override cache parameters for testing. */ private final CacheTestParameters cacheTestParameters = new CacheTestParameters(); private final Map> cache = new HashMap<>(); private final Map lastUsed = new HashMap<>(); - // TODO: use the project root dir. private static final File rootPath = new File(CurrentEnsoProject.get().getRootPath()); public LRUCache() { @@ -84,10 +97,11 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB return new CacheResult<>(item.stream(), item.metadata()); } + long maxFileSize = getMaxFileSize(); if (item.sizeMaybe.isPresent()) { long size = item.sizeMaybe().get(); - if (size > getMaxFileSize()) { - throw new ResponseTooLargeException(getMaxFileSize()); + if (size > maxFileSize) { + throw new ResponseTooLargeException(maxFileSize); } makeRoomFor(size); } @@ -138,13 +152,14 @@ private File downloadResponseData(String cacheKey, Item item) boolean successful = false; try { // Limit the download to getMaxFileSize(). - boolean sizeOK = Stream_Utils.limitedCopy(inputStream, outputStream, getMaxFileSize()); + long maxFileSize = getMaxFileSize(); + boolean sizeOK = Stream_Utils.limitedCopy(inputStream, outputStream, maxFileSize ); if (sizeOK) { successful = true; return temp; } else { - throw new ResponseTooLargeException(getMaxFileSize()); + throw new ResponseTooLargeException(maxFileSize ); } } finally { outputStream.close(); @@ -264,7 +279,7 @@ private long calculateTotalCacheSize() { case TotalCacheLimit.Percentage percentage -> { long usableSpace = getUsableDiskSpace(); long totalCacheSize = (long) (percentage.percentage() * usableSpace); - yield Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE, totalCacheSize); + yield Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); } }; } From ad57bd74b0e53503a2f08a1a1b7bf3c5593aa107 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 12:08:45 -0500 Subject: [PATCH 09/40] docs, test for changing disk space --- .../java/org/enso/base/cache/LRUCache.java | 28 +++++++++++++++++-- .../org/enso/base/cache/TotalCacheLimit.java | 5 ++++ test/Table_Tests/src/IO/Fetch_Spec.enso | 3 ++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 49fa2aaccd4e..3781ccb1e57c 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -31,15 +31,30 @@ * deleting entries to make space for new ones. All cache files are set to be deleted automatically * on JVM exit. * + * Limits should be set with environment variables: + * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS -- single file size, in megs + * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT -- total cache size, in megs or percentage of free disk space + * + * Examples: + * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS=20 + * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=200 + * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=50% + * * @param Additional metadata to associate with the data. */ public class LRUCache { private static final Logger logger = Logger.getLogger(LRUCache.class.getName()); - /** Default value for the largest file size allowed. */ + /** + * Default value for the largest file size allowed. + * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS environment variable. + */ private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; - /** Default value for the percentage of free disk space to use as a limit on the total cache size. */ + /** + * Default value for the percentage of free disk space to use as a limit on the total cache size. + * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT environment variable. + */ private static final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; /** @@ -53,7 +68,14 @@ public class LRUCache { * requested through this cache, a ResponseTooLargeException is thrown. */ private final long maxFileSize; - /** Limits the total size of all files in the cache. */ + + /** + * Limits the total size of all files in the cache. + * + * This value can depend on free disk space, so it is not resolved to a + * maximum byte count at initialization time, but recalculated during each + * file cleanup. + */ private final TotalCacheLimit.Limit totalCacheLimit; /** Used to override cache parameters for testing. */ diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java index 20a4b6cd0929..db63a58d170e 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -3,7 +3,12 @@ import java.text.DecimalFormat; import java.text.ParsePosition; +/** + * Represents a limit on the total size of an LRUCache, either as a fixed byte + * count or as a percentage of available disk space. + */ public class TotalCacheLimit { + /** Parse the limit specification string into either a Megs or Percentage value. */ public static Limit parse(String limitString) throws NumberFormatException { Number percentageNumber = tryPercentage(limitString); if (percentageNumber != null) { diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index ffef554d1aff..304d74c8d67f 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -463,6 +463,9 @@ add_specs suite_builder = lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (300 * 1024 * 1024 * 1024) lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024 * 1024) lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (400 * 1024 * 1024 * 1024) + lru_cache.getMaxTotalCacheSize . should_equal (40 * 1024 * 1024 * 1024) + lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 group_builder.specify "Toital cache size should not go over the hard-coded maximum" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50%" <| From 5742258767db6cfe7758a61706eaa7ff995f5782 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 12:09:15 -0500 Subject: [PATCH 10/40] doc --- test/Table_Tests/src/IO/Fetch_Spec.enso | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 304d74c8d67f..5b021b71ea63 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -456,7 +456,7 @@ add_specs suite_builder = lru_cache = LRUCache.new lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) - group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable" <| + group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable, and track changes to available disk space." <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| Panic.with_finalizer reset_disk_space <| lru_cache = LRUCache.new From 859459ac763a4177380e5590f639ec379c544d32 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 12:12:39 -0500 Subject: [PATCH 11/40] doc --- std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 3781ccb1e57c..8dcb3bcf8517 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -295,6 +295,10 @@ public long getMaxTotalCacheSize() { return cacheTestParameters.getMaxTotalCacheSizeOverrideTestOnly().orElse(calculateTotalCacheSize()); } + /** + * Calculate the max total cache size, using the current limit but also + * constraining the result to the upper bound. + */ private long calculateTotalCacheSize() { return switch (totalCacheLimit) { case TotalCacheLimit.Megs megs -> (long) (megs.megs() * 1024 * 1024); From 875aa1f0c929dd9f9ff2ed2450ff66a53ab7443a Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 12:28:37 -0500 Subject: [PATCH 12/40] prevent raising test disk space --- .../java/org/enso/base/cache/LRUCache.java | 19 ++++++++++++++----- test/Table_Tests/src/IO/Fetch_Spec.enso | 12 ++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 8dcb3bcf8517..3ea2936144b9 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -367,15 +367,16 @@ public interface ItemBuilder { private final Comparator>> cacheEntryLRUComparator = Comparator.comparing(me -> lastUsed.get(me.getKey())); - /** A set of parameters that can be used to modify cache settings for testing purposes. */ + /** + * A set of parameters that can be used to modify cache settings for testing purposes. + * + * Since these are callable from Enso, size limits cannot be set to a value + * larger than the real limit. + */ public class CacheTestParameters { /** This value is used for the current time when testing TTL expiration logic. */ private Optional nowOverrideTestOnly = Optional.empty(); - /** - * Used for testing file and cache size limits. These cannot be set to values larger than the - * real limits. - */ private Optional maxFileSizeOverrideTestOnly = Optional.empty(); private Optional maxTotalCacheSizeOverrideTestOnly = Optional.empty(); @@ -434,6 +435,12 @@ public Optional getUsableDiskSpaceOverrideTestOnly() { } public void setUsableDiskSpaceOverrideTestOnly(long usableDiskSpaceOverrideTestOnly_) { + long usableDiskSpace = rootPath.getUsableSpace(); + if (usableDiskSpaceOverrideTestOnly_ > usableDiskSpace) { + throw new IllegalArgumentException( + "Cannot set the (test-only) usable disk space to more than the allowed limit of " + + usableDiskSpace); + } usableDiskSpaceOverrideTestOnly = Optional.of(usableDiskSpaceOverrideTestOnly_); } @@ -442,6 +449,7 @@ public void clearUsableDiskSpaceOverrideTestOnly() { } } + /** Uses the environment variable if set, otherwise uses a default. */ private static long calcMaxFileSize() { String maxFileSizeMegsVar = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); if (maxFileSizeMegsVar != null) { @@ -452,6 +460,7 @@ private static long calcMaxFileSize() { } } + /** Uses the environment variable if set, otherwise uses a default percentage. */ private static TotalCacheLimit.Limit calcTotalCacheLimit() { String limitVar = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); if (limitVar != null) { diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 5b021b71ea63..a35bb103b99b 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -21,6 +21,7 @@ from enso_dev.Base_Tests.Network.Http.Http_Test_Setup import base_url_with_slash import project.Util +polyglot java import java.io.File as Java_File polyglot java import java.lang.IllegalArgumentException polyglot java import java.lang.NumberFormatException polyglot java import org.enso.base.cache.LRUCache @@ -440,6 +441,9 @@ add_specs suite_builder = EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument Test.expect_panic IllegalArgumentException <| EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument + Test.expect_panic IllegalArgumentException <| + usable_disk_space = Java_File.new enso_project.root_path . getUsableSpace + EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly usable_disk_space+1 . should_fail_with Illegal_Argument group_builder.specify "Cache limits should have defaults" <| lru_cache = EnsoSecretHelper.getCache.getLRUCache @@ -460,11 +464,11 @@ add_specs suite_builder = Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| Panic.with_finalizer reset_disk_space <| lru_cache = LRUCache.new - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (300 * 1024 * 1024 * 1024) - lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024 * 1024) + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 300 + lru_cache.getMaxTotalCacheSize . should_equal 30 lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (400 * 1024 * 1024 * 1024) - lru_cache.getMaxTotalCacheSize . should_equal (40 * 1024 * 1024 * 1024) + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 400 + lru_cache.getMaxTotalCacheSize . should_equal 40 lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 group_builder.specify "Toital cache size should not go over the hard-coded maximum" <| From 36b3867ea1d60ed36fbc3d70c065cb5ce9efcb4c Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 14:26:46 -0500 Subject: [PATCH 13/40] upper bound test --- .../src/main/java/org/enso/base/cache/LRUCache.java | 6 +++--- test/Table_Tests/src/IO/Fetch_Spec.enso | 10 ++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 3ea2936144b9..be6d32b1abce 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -300,14 +300,14 @@ public long getMaxTotalCacheSize() { * constraining the result to the upper bound. */ private long calculateTotalCacheSize() { - return switch (totalCacheLimit) { + var totalCacheSize = switch (totalCacheLimit) { case TotalCacheLimit.Megs megs -> (long) (megs.megs() * 1024 * 1024); case TotalCacheLimit.Percentage percentage -> { long usableSpace = getUsableDiskSpace(); - long totalCacheSize = (long) (percentage.percentage() * usableSpace); - yield Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); + yield (long) (percentage.percentage() * usableSpace); } }; + return Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); } private long getUsableDiskSpace() { diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index a35bb103b99b..058a01d5e8ea 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -471,12 +471,10 @@ add_specs suite_builder = lru_cache.getMaxTotalCacheSize . should_equal 40 lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 - group_builder.specify "Toital cache size should not go over the hard-coded maximum" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50%" <| - Panic.with_finalizer reset_disk_space <| - lru_cache = LRUCache.new - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly (500 * 1024 * 1024 * 1024) - lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) + group_builder.specify "Total cache size should not go over the hard-coded maximum" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| + lru_cache = LRUCache.new + lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| From e5dc3a5ede12af0fdc1848422529f2bb897258c6 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 14:43:07 -0500 Subject: [PATCH 14/40] wip --- test/Table_Tests/src/IO/Fetch_Spec.enso | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 058a01d5e8ea..bb670498d9a0 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -224,12 +224,12 @@ add_specs suite_builder = group_builder.specify "Data.download is not affected by caching limits" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" - Panic.with_finalizer reset_size_limits <| + file_deleter = target_file.delete_if_exists + Panic.with_finalizer reset_size_limits <| Panic.with_finalizer file_deleter <| EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 120 EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 Data.download base_url_with_slash+"test_download?length=200" target_file target_file.read.length . should_equal 200 - target_file.delete_if_exists group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| HTTP.clear_response_cache From 878d992a27d79ac1a7b710359c58947aca4a41b0 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 5 Nov 2024 14:55:10 -0500 Subject: [PATCH 15/40] wip --- test/Table_Tests/src/IO/Fetch_Spec.enso | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index bb670498d9a0..f71c256068ab 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -374,7 +374,6 @@ add_specs suite_builder = EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 download 110 . should_fail_with (Response_Too_Large.Error 100) - group_builder.specify "Will not cache a file without a content length, but which is greater than the single file limit" pending=pending_has_url <| Test.with_retries <| HTTP.clear_response_cache Panic.with_finalizer reset_size_limits <| From 0d9da62b52d1204728a1d1a999945a1487ef91c0 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 11:10:43 -0600 Subject: [PATCH 16/40] wip --- .../org/enso/base/cache/DiskSpaceGetter.java | 14 ++ .../java/org/enso/base/cache/LRUCache.java | 195 ++---------------- .../org/enso/base/cache/LRUCacheSettings.java | 71 +++++++ .../java/org/enso/base/cache/Mockable.java | 25 +++ .../java/org/enso/base/cache/NowGetter.java | 9 + .../enso_cloud/EnsoHTTPResponseCache.java | 13 +- 6 files changed, 152 insertions(+), 175 deletions(-) create mode 100644 std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java create mode 100644 std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java create mode 100644 std-bits/base/src/main/java/org/enso/base/cache/Mockable.java create mode 100644 std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java diff --git a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java new file mode 100644 index 000000000000..12f01ff0f7d9 --- /dev/null +++ b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java @@ -0,0 +1,14 @@ +package org.enso.base.cache; + +import java.io.File; +import org.enso.base.CurrentEnsoProject; + +public class DiskSpaceGetter extends Mockable { + public DiskSpaceGetter() { + this(() -> getRootPath().getUsableSpace()); + } + + private static File getRootPath() { + File(CurrentEnsoProject.get().getRootPath()); + } +} diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index be6d32b1abce..af0d5179f3fd 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -20,8 +20,6 @@ import java.util.logging.Logger; import java.util.stream.Collectors; -import org.enso.base.CurrentEnsoProject; -import org.enso.base.Environment_Utils; import org.enso.base.Stream_Utils; /** @@ -45,54 +43,33 @@ public class LRUCache { private static final Logger logger = Logger.getLogger(LRUCache.class.getName()); - /** - * Default value for the largest file size allowed. - * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS environment variable. - */ - private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; - - /** - * Default value for the percentage of free disk space to use as a limit on the total cache size. - * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT environment variable. - */ - private static final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; - /** * An upper limit on the total cache size. If the cache size limit specified * by the other parameters goes over this value, then this value is used. */ private static final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND = 100L * 1024 * 1024 * 1024; - /** - * Maximum size allowed for a single file. If a file larger than this is - * requested through this cache, a ResponseTooLargeException is thrown. - */ - private final long maxFileSize; - - /** - * Limits the total size of all files in the cache. - * - * This value can depend on free disk space, so it is not resolved to a - * maximum byte count at initialization time, but recalculated during each - * file cleanup. - */ - private final TotalCacheLimit.Limit totalCacheLimit; - /** Used to override cache parameters for testing. */ - private final CacheTestParameters cacheTestParameters = new CacheTestParameters(); - private final Map> cache = new HashMap<>(); private final Map lastUsed = new HashMap<>(); - private static final File rootPath = new File(CurrentEnsoProject.get().getRootPath()); + /** Defines the per-file and total cache size limits. */ + private final LRUCacheSettings settings; + + /** Used to get the current time; mockable. */ + private final NowGetter nowGetter; + + /** Used to get the current free disk space; mockable. */ + private final DiskSpaceGetter diskSpaceGetter; public LRUCache() { - this(calcMaxFileSize(), calcTotalCacheLimit()); + this(LRUCache.getDefault(), new NowGetter(), new DiskSpaceGetter()); } - public LRUCache(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) { - this.maxFileSize = maxFileSize; - this.totalCacheLimit = totalCacheLimit; + public LRUCache(LRUCacheSettings settings, NowGetter nowGetter, DiskSpaceGetter diskSpaceGetter) { + this.settings = settings; + this.nowGetter = nowGetter; + this.diskSpaceGetter = diskSpaceGetter; } public CacheResult getResult(ItemBuilder itemBuilder) @@ -119,7 +96,7 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB return new CacheResult<>(item.stream(), item.metadata()); } - long maxFileSize = getMaxFileSize(); + long maxFileSize = settings.getMaxFileSize(); if (item.sizeMaybe.isPresent()) { long size = item.sizeMaybe().get(); if (size > maxFileSize) { @@ -133,7 +110,7 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB File responseData = downloadResponseData(cacheKey, item); M metadata = item.metadata(); long size = responseData.length(); - ZonedDateTime expiry = getNow().plus(Duration.ofSeconds(item.ttl().get())); + ZonedDateTime expiry = nowGetter.get().plus(Duration.ofSeconds(item.ttl().get())); // Create a cache entry. var cacheEntry = new CacheEntry<>(responseData, metadata, size, expiry); @@ -174,7 +151,7 @@ private File downloadResponseData(String cacheKey, Item item) boolean successful = false; try { // Limit the download to getMaxFileSize(). - long maxFileSize = getMaxFileSize(); + long maxFileSize = settings.getMaxFileSize(); boolean sizeOK = Stream_Utils.limitedCopy(inputStream, outputStream, maxFileSize ); if (sizeOK) { @@ -195,12 +172,12 @@ private File downloadResponseData(String cacheKey, Item item) /** Mark the entry with the current time, to maintain LRU data. */ private void markCacheEntryUsed(String cacheKey) { - lastUsed.put(cacheKey, getNow()); + lastUsed.put(cacheKey, nowGetter.get()); } /** Remove all cache entries (and their files) that have passed their TTL. */ private void removeStaleEntries() { - var now = getNow(); + var now = nowGetter.get(); removeCacheEntriesByPredicate(e -> e.expiry().isBefore(now)); } @@ -281,39 +258,21 @@ private long getTotalCacheSize() { return cache.values().stream().collect(Collectors.summingLong(e -> e.size())); } - // Public for testing. - public long getMaxFileSize() { - return cacheTestParameters.getMaxFileSizeOverrideTestOnly().orElse(maxFileSize); - } - - public TotalCacheLimit.Limit getTotalCacheLimit() { - return totalCacheLimit; - } - - // Public for testing. - public long getMaxTotalCacheSize() { - return cacheTestParameters.getMaxTotalCacheSizeOverrideTestOnly().orElse(calculateTotalCacheSize()); - } - /** * Calculate the max total cache size, using the current limit but also * constraining the result to the upper bound. */ - private long calculateTotalCacheSize() { - var totalCacheSize = switch (totalCacheLimit) { + public long getMaxTotalCacheSize() { + var totalCacheSize = switch (settings.getTotalCacheLimit()) { case TotalCacheLimit.Megs megs -> (long) (megs.megs() * 1024 * 1024); case TotalCacheLimit.Percentage percentage -> { - long usableSpace = getUsableDiskSpace(); + long usableSpace = diskSpaceGetter.get(); yield (long) (percentage.percentage() * usableSpace); } }; return Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); } - private long getUsableDiskSpace() { - return cacheTestParameters.getUsableDiskSpaceOverrideTestOnly().orElse(rootPath.getUsableSpace()); - } - public int getNumEntries() { return cache.size(); } @@ -323,15 +282,6 @@ public List getFileSizesTestOnly() { cache.values().stream().map(CacheEntry::size).collect(Collectors.toList())); } - private ZonedDateTime getNow() { - return cacheTestParameters.getNowOverrideTestOnly().orElse(ZonedDateTime.now()); - } - - /** Return a set of parameters that can be used to modify settings for testing purposes. */ - public CacheTestParameters getCacheTestParameters() { - return cacheTestParameters; - } - private record CacheEntry(File responseData, M metadata, long size, ZonedDateTime expiry) {} /** @@ -366,107 +316,4 @@ public interface ItemBuilder { private final Comparator>> cacheEntryLRUComparator = Comparator.comparing(me -> lastUsed.get(me.getKey())); - - /** - * A set of parameters that can be used to modify cache settings for testing purposes. - * - * Since these are callable from Enso, size limits cannot be set to a value - * larger than the real limit. - */ - public class CacheTestParameters { - /** This value is used for the current time when testing TTL expiration logic. */ - private Optional nowOverrideTestOnly = Optional.empty(); - - private Optional maxFileSizeOverrideTestOnly = Optional.empty(); - - private Optional maxTotalCacheSizeOverrideTestOnly = Optional.empty(); - - private Optional usableDiskSpaceOverrideTestOnly = Optional.empty(); - - public Optional getNowOverrideTestOnly() { - return nowOverrideTestOnly; - } - - public void setNowOverrideTestOnly(ZonedDateTime nowOverride) { - nowOverrideTestOnly = Optional.of(nowOverride); - } - - public void clearNowOverrideTestOnly() { - nowOverrideTestOnly = Optional.empty(); - } - - public Optional getMaxFileSizeOverrideTestOnly() { - return maxFileSizeOverrideTestOnly; - } - - public void setMaxFileSizeOverrideTestOnly(long maxFileSizeOverrideTestOnly_) { - if (maxFileSizeOverrideTestOnly_ > maxFileSize) { - throw new IllegalArgumentException( - "Cannot set the (test-only) maximum file size to more than the allowed limit of " - + maxFileSize); - } - maxFileSizeOverrideTestOnly = Optional.of(maxFileSizeOverrideTestOnly_); - } - - public void clearMaxFileSizeOverrideTestOnly() { - maxFileSizeOverrideTestOnly = Optional.empty(); - } - - public Optional getMaxTotalCacheSizeOverrideTestOnly() { - return maxTotalCacheSizeOverrideTestOnly; - } - - public void setMaxTotalCacheSizeOverrideTestOnly(long maxTotalCacheSizeOverrideTestOnly_) { - long maxTotalCacheSize = getMaxTotalCacheSize(); - if (maxTotalCacheSizeOverrideTestOnly_ > maxTotalCacheSize) { - throw new IllegalArgumentException( - "Cannot set the (test-only) total cache size to more than the allowed limit of " - + maxTotalCacheSize); - } - maxTotalCacheSizeOverrideTestOnly = Optional.of(maxTotalCacheSizeOverrideTestOnly_); - } - - public void clearMaxTotalCacheSizeOverrideTestOnly() { - maxTotalCacheSizeOverrideTestOnly = Optional.empty(); - } - - public Optional getUsableDiskSpaceOverrideTestOnly() { - return usableDiskSpaceOverrideTestOnly; - } - - public void setUsableDiskSpaceOverrideTestOnly(long usableDiskSpaceOverrideTestOnly_) { - long usableDiskSpace = rootPath.getUsableSpace(); - if (usableDiskSpaceOverrideTestOnly_ > usableDiskSpace) { - throw new IllegalArgumentException( - "Cannot set the (test-only) usable disk space to more than the allowed limit of " - + usableDiskSpace); - } - usableDiskSpaceOverrideTestOnly = Optional.of(usableDiskSpaceOverrideTestOnly_); - } - - public void clearUsableDiskSpaceOverrideTestOnly() { - usableDiskSpaceOverrideTestOnly = Optional.empty(); - } - } - - /** Uses the environment variable if set, otherwise uses a default. */ - private static long calcMaxFileSize() { - String maxFileSizeMegsVar = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); - if (maxFileSizeMegsVar != null) { - double maxFileSizeMegs = Double.parseDouble(maxFileSizeMegsVar); - return (long) (maxFileSizeMegs * 1024 * 1024); - } else { - return DEFAULT_MAX_FILE_SIZE; - } - } - - /** Uses the environment variable if set, otherwise uses a default percentage. */ - private static TotalCacheLimit.Limit calcTotalCacheLimit() { - String limitVar = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); - if (limitVar != null) { - return TotalCacheLimit.parse(limitVar); - } else { - return new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); - } - } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java new file mode 100644 index 000000000000..16385bc0759f --- /dev/null +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -0,0 +1,71 @@ +package org.enso.base.cache; + +import org.enso.base.Environment_Utils; + +public class LRUCacheSettings { + /** + * Default value for the largest file size allowed. + * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS environment variable. + */ + private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; + + /** + * Default value for the percentage of free disk space to use as a limit on the total cache size. + * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT environment variable. + */ + private static final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; + + /** + * Maximum size allowed for a single file. If a file larger than this is + * requested through this cache, a ResponseTooLargeException is thrown. + */ + private final long maxFileSize; + + /** + * Limits the total size of all files in the cache. + * + * This value can depend on free disk space, so it is not resolved to a + * maximum byte count at initialization time, but recalculated during each + * file cleanup. + */ + private final TotalCacheLimit.Limit totalCacheLimit; + + public LRUCacheSettings(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) { + this.maxFileSize = maxFileSize; + this.totalCacheLimit = totalCacheLimit; + } + + public LRUCacheSettings(String maxFileSizeSpec, String totalCacheLimitSpec) { + this(parseMaxFileSize(maxFileSizeSpec), parseTotalCacheLimit(totalCacheLimitSpec)); + } + + /** Uses defaults if the vars are not set. */ + public static LRUCacheSettings getDefault() { + String maxFileSizeSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); + String totalCacheLimitSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); + if (maxFileSizeSpec != null && totalCacheLimitSpec != null) { + return new LRUCacheSettings(maxFileSizeSpec, totalCacheLimitSpec); + } else { + return new LRUCacheSettings(DEFAULT_MAX_FILE_SIZE, DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + } + } + + public long getMaxFileSize() { + return maxFileSize; + } + + public TotalCacheSize.Limit getTotalCacheLimit() { + return totalCacheLimit; + } + + /** Uses the environment variable if set, otherwise uses a default. */ + private static long parseMaxFileSize(String maxFileSizeSpec) { + double maxFileSizeMegs = Double.parseDouble(maxFileSizeSpec); + return (long) (maxFileSizeMegs * 1024 * 1024); + } + + /** Uses the environment variable if set, otherwise uses a default percentage. */ + private static TotalCacheLimit.Limit parseTotalCacheLimit() { + return TotalCacheLimit.parse(totalCacheLimitSpec ); + } +} \ No newline at end of file diff --git a/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java b/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java new file mode 100644 index 000000000000..e2a0456de46d --- /dev/null +++ b/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java @@ -0,0 +1,25 @@ +package org.enso.base.cache; + +import java.util.Optional; +import java.util.function.Supplier; + +public class Mockable { + private Supplier supplier; + private Optional override = Optional.empty(); + + public Mockable(Supplier supplier) { + this.supplier = supplier; + } + + public void mocked(T t) { + this.override = Optional.of(t); + } + + public void unmocked() { + this.override = Optional.empty(); + } + + public T get() { + return override.orElse(supplier.get()); + } +} diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java new file mode 100644 index 000000000000..eb1c44d50975 --- /dev/null +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -0,0 +1,9 @@ +package org.enso.base.cache; + +import java.time.ZonedDateTime; + +public class NowGetter extends Mockable { + public NowGetter() { + this(() -> ZonedDateTime.now()); + } +} diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index 90f407d7f0cb..07bc1a16bc32 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -24,12 +24,13 @@ public class EnsoHTTPResponseCache { // Public for testing. public EnsoHTTPResponseCache() { + resetToDefaultLRUCache(); } // 1 year. private final int DEFAULT_TTL_SECONDS = 31536000; - private final LRUCache lruCache = new LRUCache<>(); + private final LRUCache lruCache; public EnsoHttpResponse makeRequest(RequestMaker requestMaker) throws IOException, InterruptedException, ResponseTooLargeException { @@ -142,6 +143,16 @@ public LRUCache.CacheTestParameters getCacheTestParameters() { return lruCache.getCacheTestParameters(); } + /** Public for testing. */ + public void setLRUCache(LRUCache lruCache) { + this.lruCache.clear(); + this.lruCache = lruCache; + } + + public void resetToDefaultLRUCache() { + setLRUCache(new LRUCache<>()); + } + public LRUCache getLRUCache() { return lruCache; } From d62ffa9e8029db666b7e4c23504a2ebe0b22d9ed Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 11:58:36 -0600 Subject: [PATCH 17/40] wip --- .../enso_cloud/EnsoHTTPResponseCache.java | 26 +-- test/Table_Tests/src/IO/Fetch_Spec.enso | 157 ++++++++++-------- 2 files changed, 90 insertions(+), 93 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index 07bc1a16bc32..06e63e205b75 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -126,21 +126,9 @@ private Integer getMaxAge(HttpHeaders headers) { return maxAge; } - public void clear() { - lruCache.clear(); - } - - public int getNumEntries() { - return lruCache.getNumEntries(); - } - - public List getFileSizesTestOnly() { - return lruCache.getFileSizesTestOnly(); - } - - /** Return a set of parameters that can be used to modify settings for testing purposes. */ - public LRUCache.CacheTestParameters getCacheTestParameters() { - return lruCache.getCacheTestParameters(); + /** Public for testing. */ + public LRUCache getLRUCache() { + return lruCache; } /** Public for testing. */ @@ -149,14 +137,6 @@ public void setLRUCache(LRUCache lruCache) { this.lruCache = lruCache; } - public void resetToDefaultLRUCache() { - setLRUCache(new LRUCache<>()); - } - - public LRUCache getLRUCache() { - return lruCache; - } - public interface RequestMaker { /** Executes the HTTP request and returns the response. */ EnsoHttpResponse makeRequest() throws IOException, InterruptedException; diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index f71c256068ab..f04a5718ea56 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -24,7 +24,9 @@ import project.Util polyglot java import java.io.File as Java_File polyglot java import java.lang.IllegalArgumentException polyglot java import java.lang.NumberFormatException +polyglot java import org.enso.base.cache.DiskSpaceGetter polyglot java import org.enso.base.cache.LRUCache +polyglot java import org.enso.base.cache.NowGetter polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper main filter=Nothing = @@ -110,27 +112,43 @@ add_specs suite_builder = r4.should_equal (Table.from_rows ["Column 1"] [["A,B"], ["1,x"], ["3,y"]]) suite_builder.group "Response caching" pending=pending_has_url group_builder-> + get_lru_cache = + EnsoSecretHelper.getCache.getLRUCache + get_num_response_cache_entries = - EnsoSecretHelper.getCache.getNumEntries + get_lru_cache.getNumEntries + with_counts ~action = before_count = get_num_response_cache_entries action after_count = get_num_response_cache_entries [before_count, after_count] - reset_size_limits = - EnsoSecretHelper.getCache.getCacheTestParameters.clearMaxFileSizeOverrideTestOnly - EnsoSecretHelper.getCache.getCacheTestParameters.clearMaxTotalCacheSizeOverrideTestOnly - reset_disk_space = - EnsoSecretHelper.getCache.getCacheTestParameters.clearUsableDiskSpaceOverrideTestOnly - expect_counts expected_counts ~action = counts = with_counts action counts . should_equal expected_counts frames_to_skip=1 get_cache_file_sizes : Vector Integer get_cache_file_sizes -> Vector Integer = - Vector.from_polyglot_array EnsoSecretHelper.getCache.getFileSizesTestOnly . sort Sort_Direction.Ascending + Vector.from_polyglot_array EnsoSecretHelper.getCache.getLRUCache.getFileSizesTestOnly . sort Sort_Direction.Ascending + + # Craetes a new cache each time, then resets it at the end + with_lru_cache lru_cache action = + reset = EnsoSecretHelper.getCache.setLRUCache LRUCache.new + Panic.with_finalizer reset action + + # Craetes a new cache each time, then resets it at the end + with_config max_file_size total_cache_size action = + now_getter = NowGetter.new + disk_space_getter = DiskSpaceGetter.new + lru_cache_settings = LRUCacheSettings.new max_file_size total_cache_size + lru_cache = LRUCache.new lru_cache_settings now_getter disk_space_getter + with_lru_cache lru_cache (action now_getter disk_space_getter) + + # Craetes a new cache each time, then resets it at the end + with_default_cache action = + lru_cache = LRUCache.new + with_lru_cache lru_cache action url0 = base_url_with_slash+'test_download?max-age=16&length=10' url1 = base_url_with_slash+'test_download?max-age=16&length=20' @@ -144,83 +162,82 @@ add_specs suite_builder = results.distinct.length . should_equal 1 group_builder.specify "Cache should return the same repsonse" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - check_same_results <| - HTTP.fetch url0 . decode_as_text - get_num_response_cache_entries . should_equal 1 - check_same_results <| - HTTP.fetch url1 . decode_as_text - get_num_response_cache_entries . should_equal 2 - - HTTP.clear_response_cache - - HTTP.fetch url0 cache_policy=Cache_Policy.Use_Cache . decode_as_text - HTTP.fetch url0 cache_policy=Cache_Policy.Use_Cache . decode_as_text - url1_body_1 = HTTP.fetch url1 cache_policy=Cache_Policy.Use_Cache . decode_as_text - HTTP.fetch url1 cache_policy=Cache_Policy.Use_Cache . decode_as_text . should_equal url1_body_1 - get_num_response_cache_entries . should_equal 2 - - HTTP.clear_response_cache - - url0_body_2 = HTTP.fetch url0 cache_policy=Cache_Policy.No_Cache . decode_as_text - HTTP.fetch url0 cache_policy=Cache_Policy.No_Cache . decode_as_text . should_not_equal url0_body_2 - url1_body_2 = HTTP.fetch url1 cache_policy=Cache_Policy.No_Cache . decode_as_text - HTTP.fetch url1 cache_policy=Cache_Policy.No_Cache . decode_as_text . should_not_equal url1_body_2 - get_num_response_cache_entries . should_equal 0 + with_default_cache <| + check_same_results <| + HTTP.fetch url0 . decode_as_text + get_num_response_cache_entries . should_equal 1 + check_same_results <| + HTTP.fetch url1 . decode_as_text + get_num_response_cache_entries . should_equal 2 + + with_default_cache <| + HTTP.fetch url0 cache_policy=Cache_Policy.Use_Cache . decode_as_text + HTTP.fetch url0 cache_policy=Cache_Policy.Use_Cache . decode_as_text + url1_body_1 = HTTP.fetch url1 cache_policy=Cache_Policy.Use_Cache . decode_as_text + HTTP.fetch url1 cache_policy=Cache_Policy.Use_Cache . decode_as_text . should_equal url1_body_1 + get_num_response_cache_entries . should_equal 2 + + with_default_cache <| + url0_body_2 = HTTP.fetch url0 cache_policy=Cache_Policy.No_Cache . decode_as_text + HTTP.fetch url0 cache_policy=Cache_Policy.No_Cache . decode_as_text . should_not_equal url0_body_2 + url1_body_2 = HTTP.fetch url1 cache_policy=Cache_Policy.No_Cache . decode_as_text + HTTP.fetch url1 cache_policy=Cache_Policy.No_Cache . decode_as_text . should_not_equal url1_body_2 + get_num_response_cache_entries . should_equal 0 group_builder.specify "Cache should handle many entries" pending=pending_has_url <| Test.with_retries <| count = 20 - HTTP.clear_response_cache - check_same_results <| - 0.up_to count . map i-> - HTTP.fetch base_url_with_slash+"test_download?length="+i.to_text . decode_as_text - get_num_response_cache_entries . should_equal count + with_default_cache <| + check_same_results <| + 0.up_to count . map i-> + HTTP.fetch base_url_with_slash+"test_download?length="+i.to_text . decode_as_text + get_num_response_cache_entries . should_equal count - HTTP.clear_response_cache - check_same_results <| - 0.up_to count . each i-> - headers = [Header.new "A-Header" "a-header-value-"+i.to_text] - HTTP.fetch base_url_with_slash+"test_download?length=8" headers=headers . decode_as_text - get_num_response_cache_entries . should_equal count + with_default_cache <| + check_same_results <| + 0.up_to count . each i-> + headers = [Header.new "A-Header" "a-header-value-"+i.to_text] + HTTP.fetch base_url_with_slash+"test_download?length=8" headers=headers . decode_as_text + get_num_response_cache_entries . should_equal count group_builder.specify "Cache policy should work for HTTP.fetch" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - expect_counts [0, 0] <| - HTTP.fetch url0 cache_policy=Cache_Policy.No_Cache - HTTP.fetch url1 cache_policy=Cache_Policy.No_Cache - expect_counts [0, 2] <| - HTTP.fetch url0 cache_policy=Cache_Policy.Use_Cache - HTTP.fetch url1 cache_policy=Cache_Policy.Use_Cache - HTTP.clear_response_cache - expect_counts [0, 2] <| - HTTP.fetch url0 - HTTP.fetch url1 + with_default_cache <| + expect_counts [0, 0] <| + HTTP.fetch url0 cache_policy=Cache_Policy.No_Cache + HTTP.fetch url1 cache_policy=Cache_Policy.No_Cache + expect_counts [0, 2] <| + HTTP.fetch url0 cache_policy=Cache_Policy.Use_Cache + HTTP.fetch url1 cache_policy=Cache_Policy.Use_Cache + + with_default_cache <| + expect_counts [0, 2] <| + HTTP.fetch url0 + HTTP.fetch url1 group_builder.specify "Cache policy should work for Data.fetch" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - expect_counts [0, 0] <| - Data.fetch url0 cache_policy=Cache_Policy.No_Cache - Data.fetch url1 cache_policy=Cache_Policy.No_Cache - expect_counts [0, 2] <| - Data.fetch url0 cache_policy=Cache_Policy.Use_Cache - Data.fetch url1 cache_policy=Cache_Policy.Use_Cache - HTTP.clear_response_cache - expect_counts [0, 2] <| - Data.fetch url0 - Data.fetch url1 + with_default_cache <| + expect_counts [0, 0] <| + Data.fetch url0 cache_policy=Cache_Policy.No_Cache + Data.fetch url1 cache_policy=Cache_Policy.No_Cache + expect_counts [0, 2] <| + Data.fetch url0 cache_policy=Cache_Policy.Use_Cache + Data.fetch url1 cache_policy=Cache_Policy.Use_Cache + + with_default_cache <| + expect_counts [0, 2] <| + Data.fetch url0 + Data.fetch url1 group_builder.specify "Should not cache Data.download" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" - HTTP.clear_response_cache - target_file.delete_if_exists + with_default_cache <| + target_file.delete_if_exists - Data.download url0 target_file - get_num_response_cache_entries . should_equal 0 + Data.download url0 target_file + get_num_response_cache_entries . should_equal 0 - target_file.delete_if_exists + target_file.delete_if_exists group_builder.specify "Data.download is not affected by caching limits" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" From 0321544f1ce08022c2968079e9f3a494be22ea77 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 14:30:51 -0600 Subject: [PATCH 18/40] one passes --- .../org/enso/base/cache/DiskSpaceGetter.java | 6 +- .../java/org/enso/base/cache/LRUCache.java | 4 +- .../org/enso/base/cache/LRUCacheSettings.java | 6 +- .../java/org/enso/base/cache/NowGetter.java | 2 +- .../org/enso/base/cache/TotalCacheLimit.java | 12 +- .../enso_cloud/EnsoHTTPResponseCache.java | 5 +- test/Table_Tests/src/IO/Fetch_Spec.enso | 529 +++++++++--------- 7 files changed, 285 insertions(+), 279 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java index 12f01ff0f7d9..1e2e77107c7e 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java @@ -3,12 +3,12 @@ import java.io.File; import org.enso.base.CurrentEnsoProject; -public class DiskSpaceGetter extends Mockable { +public class DiskSpaceGetter extends Mockable { public DiskSpaceGetter() { - this(() -> getRootPath().getUsableSpace()); + super(() -> getRootPath().getUsableSpace()); } private static File getRootPath() { - File(CurrentEnsoProject.get().getRootPath()); + return new File(CurrentEnsoProject.get().getRootPath()); } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index af0d5179f3fd..caba59003bc6 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -63,7 +63,7 @@ public class LRUCache { private final DiskSpaceGetter diskSpaceGetter; public LRUCache() { - this(LRUCache.getDefault(), new NowGetter(), new DiskSpaceGetter()); + this(LRUCacheSettings.getDefault(), new NowGetter(), new DiskSpaceGetter()); } public LRUCache(LRUCacheSettings settings, NowGetter nowGetter, DiskSpaceGetter diskSpaceGetter) { @@ -264,7 +264,7 @@ private long getTotalCacheSize() { */ public long getMaxTotalCacheSize() { var totalCacheSize = switch (settings.getTotalCacheLimit()) { - case TotalCacheLimit.Megs megs -> (long) (megs.megs() * 1024 * 1024); + case TotalCacheLimit.Bytes bytes -> bytes.bytes(); case TotalCacheLimit.Percentage percentage -> { long usableSpace = diskSpaceGetter.get(); yield (long) (percentage.percentage() * usableSpace); diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java index 16385bc0759f..2914dbcbed51 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -46,7 +46,7 @@ public static LRUCacheSettings getDefault() { if (maxFileSizeSpec != null && totalCacheLimitSpec != null) { return new LRUCacheSettings(maxFileSizeSpec, totalCacheLimitSpec); } else { - return new LRUCacheSettings(DEFAULT_MAX_FILE_SIZE, DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + return new LRUCacheSettings(DEFAULT_MAX_FILE_SIZE, new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE)); } } @@ -54,7 +54,7 @@ public long getMaxFileSize() { return maxFileSize; } - public TotalCacheSize.Limit getTotalCacheLimit() { + public TotalCacheLimit.Limit getTotalCacheLimit() { return totalCacheLimit; } @@ -65,7 +65,7 @@ private static long parseMaxFileSize(String maxFileSizeSpec) { } /** Uses the environment variable if set, otherwise uses a default percentage. */ - private static TotalCacheLimit.Limit parseTotalCacheLimit() { + private static TotalCacheLimit.Limit parseTotalCacheLimit(String totalCacheLimitSpec) { return TotalCacheLimit.parse(totalCacheLimitSpec ); } } \ No newline at end of file diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index eb1c44d50975..8c53b90052bb 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -4,6 +4,6 @@ public class NowGetter extends Mockable { public NowGetter() { - this(() -> ZonedDateTime.now()); + super(() -> ZonedDateTime.now()); } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java index db63a58d170e..8e993e13db09 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -8,7 +8,7 @@ * count or as a percentage of available disk space. */ public class TotalCacheLimit { - /** Parse the limit specification string into either a Megs or Percentage value. */ + /** Parse the limit specification string into either a Bytes or Percentage value. */ public static Limit parse(String limitString) throws NumberFormatException { Number percentageNumber = tryPercentage(limitString); if (percentageNumber != null) { @@ -18,13 +18,15 @@ public static Limit parse(String limitString) throws NumberFormatException { } return new Percentage(percentage); } - return new Megs(Double.parseDouble(limitString)); + double megs = Double.parseDouble(limitString); + long bytes = (long) (megs * 1024 * 1024); + return new Bytes(bytes); } - public sealed interface Limit permits Megs, Percentage {} + public sealed interface Limit permits Bytes, Percentage {} - // Specify the limit in megs. - public record Megs(double megs) implements Limit {} + // Specify the limit in bytes. + public record Bytes(long bytes) implements Limit {} // Specify the limit as a percentage of total free, usable disk space. public record Percentage(double percentage) implements Limit {} diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index 06e63e205b75..af40d57f5549 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -24,13 +24,12 @@ public class EnsoHTTPResponseCache { // Public for testing. public EnsoHTTPResponseCache() { - resetToDefaultLRUCache(); } // 1 year. private final int DEFAULT_TTL_SECONDS = 31536000; - private final LRUCache lruCache; + private LRUCache lruCache = new LRUCache<>(); public EnsoHttpResponse makeRequest(RequestMaker requestMaker) throws IOException, InterruptedException, ResponseTooLargeException { @@ -132,7 +131,7 @@ public LRUCache getLRUCache() { } /** Public for testing. */ - public void setLRUCache(LRUCache lruCache) { + public void setLRUCache(LRUCache lruCache) { this.lruCache.clear(); this.lruCache = lruCache; } diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index f04a5718ea56..26bfd5e19cf3 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -26,7 +26,9 @@ polyglot java import java.lang.IllegalArgumentException polyglot java import java.lang.NumberFormatException polyglot java import org.enso.base.cache.DiskSpaceGetter polyglot java import org.enso.base.cache.LRUCache +polyglot java import org.enso.base.cache.LRUCacheSettings polyglot java import org.enso.base.cache.NowGetter +polyglot java import org.enso.base.cache.TotalCacheLimit polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper main filter=Nothing = @@ -133,12 +135,14 @@ add_specs suite_builder = Vector.from_polyglot_array EnsoSecretHelper.getCache.getLRUCache.getFileSizesTestOnly . sort Sort_Direction.Ascending # Craetes a new cache each time, then resets it at the end - with_lru_cache lru_cache action = + with_lru_cache lru_cache ~action = reset = EnsoSecretHelper.getCache.setLRUCache LRUCache.new - Panic.with_finalizer reset action + Panic.with_finalizer reset <| + EnsoSecretHelper.getCache.setLRUCache lru_cache + action # Craetes a new cache each time, then resets it at the end - with_config max_file_size total_cache_size action = + with_config max_file_size total_cache_size ~action = now_getter = NowGetter.new disk_space_getter = DiskSpaceGetter.new lru_cache_settings = LRUCacheSettings.new max_file_size total_cache_size @@ -146,7 +150,7 @@ add_specs suite_builder = with_lru_cache lru_cache (action now_getter disk_space_getter) # Craetes a new cache each time, then resets it at the end - with_default_cache action = + with_default_cache ~action = lru_cache = LRUCache.new with_lru_cache lru_cache action @@ -242,262 +246,263 @@ add_specs suite_builder = group_builder.specify "Data.download is not affected by caching limits" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" file_deleter = target_file.delete_if_exists - Panic.with_finalizer reset_size_limits <| Panic.with_finalizer file_deleter <| - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 120 - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 - Data.download base_url_with_slash+"test_download?length=200" target_file - target_file.read.length . should_equal 200 - - group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - expect_counts [0, 0] <| - Data.post url_post (Request_Body.Text "hello world") - - group_builder.specify "HTTP request with a non-GET method should reject a cache_policy=Use_Cache argument" pending=pending_has_url <| Test.with_retries <| - request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty - HTTP.new.request request cache_policy=Cache_Policy.Use_Cache . should_fail_with Illegal_Argument - - group_builder.specify "HTTP request with a non-GET method should not reject a cache_policy=No_Cache argument" pending=pending_has_url <| Test.with_retries <| - request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty - HTTP.new.request request cache_policy=Cache_Policy.No_Cache . should_succeed - - group_builder.specify "Should be able to clear caches" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - Data.fetch url0 - get_num_response_cache_entries . should_equal 1 - HTTP.clear_response_cache - get_num_response_cache_entries . should_equal 0 - - group_builder.specify "Cache key should depend on the headers" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - expect_counts [0, 2] <| - Data.fetch url0 headers=headers0 - Data.fetch url0 headers=headers1 - Data.fetch url0 headers=headers1 - Data.fetch url0 headers=headers0 - Data.fetch url0 headers=headers0 - Data.fetch url0 headers=headers1 - - group_builder.specify "Cache key should not depend on header order" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - header0 = Header.new "Abc" "eef" - header1 = Header.new "Abc" "def" - header2 = Header.new "Ghi" "jkl" - orders = [[header0, header1, header2], [header1, header2, header0], [header2, header1, header0]] - responses = orders.map headers-> - Data.fetch url0 headers=headers . decode_as_text - get_num_response_cache_entries . should_equal 1 - responses.distinct.length . should_equal 1 - - ## Fetching the trigger uri causes stale entries to be removed, since the - uri is always different and so the caching and cleanup logic is run. - fake_now = Date_Time.now - trigger_uri_serial = Ref.new 0 - make_trigger_uri = - serial = trigger_uri_serial.get - trigger_uri_serial.modify (_ + 1) - base_url_with_slash+'test_download?max-age=10000&length=50&abc='+serial.to_text - set_time_and_get_count advance_secs = - EnsoSecretHelper.getCache.getCacheTestParameters.setNowOverrideTestOnly (fake_now + (Duration.new seconds=advance_secs)) - trigger_uri = make_trigger_uri - Data.fetch trigger_uri - get_num_response_cache_entries - fake_time_resetter = - EnsoSecretHelper.getCache.getCacheTestParameters.clearNowOverrideTestOnly - - group_builder.specify "The cache should expire stale entries" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - set_time_and_get_count 0 # Initialize fake now. - - Data.fetch base_url_with_slash+'test_download?max-age=100&length=50' - Data.fetch base_url_with_slash+'test_download?max-age=200&length=50' - Data.fetch base_url_with_slash+'test_download?max-age=200&length=51' - Data.fetch base_url_with_slash+'test_download?max-age=300&length=50' - - Panic.with_finalizer fake_time_resetter <| - ## The count will increase by 1 each time, but decrease by the - number of entries removed - set_time_and_get_count 0 . should_equal 6 - set_time_and_get_count 90 . should_equal 7 - set_time_and_get_count 110 . should_equal 7 - set_time_and_get_count 190 . should_equal 8 - set_time_and_get_count 202 . should_equal 7 - set_time_and_get_count 292 . should_equal 8 - set_time_and_get_count 301 . should_equal 8 - - group_builder.specify "The cache should use the Age response header" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - set_time_and_get_count 0 # Initialize fake now. - - Data.fetch base_url_with_slash+'test_download?max-age=100&age=50&length=50' # ttl 50 - Data.fetch base_url_with_slash+'test_download?max-age=100&age=30&length=50' # ttl 70 - Data.fetch base_url_with_slash+'test_download?max-age=120&age=50&length=50' # ttl 70 - Data.fetch base_url_with_slash+'test_download?max-age=70&&length=50' # ttl 70 - Data.fetch base_url_with_slash+'test_download?max-age=160&age=70&length=50' # ttl 90 - - Panic.with_finalizer fake_time_resetter <| - ## The count will increase by 1 each time, but decrease by the - number of entries removed - set_time_and_get_count 0 . should_equal 7 - set_time_and_get_count 40 . should_equal 8 - set_time_and_get_count 51 . should_equal 8 - set_time_and_get_count 68 . should_equal 9 - set_time_and_get_count 72 . should_equal 7 - set_time_and_get_count 88 . should_equal 8 - set_time_and_get_count 93 . should_equal 8 - - download size = - Data.fetch base_url_with_slash+'test_download?length='+size.to_text - - group_builder.specify "Will remove old cache files to keep the total cache size under the total cache size limit" pending=pending_has_url <| Test.with_retries <| - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 - - download 30 - download 50 - download 10 - get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - download 20 - get_cache_file_sizes . should_equal_ignoring_order [10, 20, 50] - download 40 - get_cache_file_sizes . should_equal_ignoring_order [10, 20, 40] - download 35 - get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] - - group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 - - download 30 - download 50 - download 10 - get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - # Use 30 again so it's considered more recently used. - download 30 - get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - download 20 - get_cache_file_sizes . should_equal_ignoring_order [10, 20, 30] - download 45 - get_cache_file_sizes . should_equal_ignoring_order [20, 30, 45] - - group_builder.specify "Will not cache a file with a content length greater than the single file limit" pending=pending_has_url <| Test.with_retries <| - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 - download 110 . should_fail_with (Response_Too_Large.Error 100) - - group_builder.specify "Will not cache a file without a content length, but which is greater than the single file limit" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 - url = base_url_with_slash+'test_download?omit-content-length=1&length=110' - Data.fetch url . should_fail_with (Response_Too_Large.Error 100) - - group_builder.specify "Should not cache if the request fails" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - HTTP.fetch url0 - get_num_response_cache_entries . should_equal 1 - HTTP.fetch base_url_with_slash+'crash' - get_num_response_cache_entries . should_equal 1 - HTTP.fetch base_url_with_slash+'nonexistent_endpoint' - get_num_response_cache_entries . should_equal 1 - - cloud_setup = Cloud_Tests_Setup.prepare - - group_builder.specify "Should work with secrets in the URI" pending=pending_has_url <| Test.with_retries <| - cloud_setup.with_prepared_environment <| - secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" - secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" - cleanup = - secret1.delete - secret2.delete - Panic.with_finalizer cleanup <| - # Requests differ only in secrets in URI. - url1 = URI.from 'https://httpbin.org/bytes/50' - . add_query_argument "arg1" secret1 - . add_query_argument "arg2" "plain value" - uri2 = URI.from 'https://httpbin.org/bytes/50' - . add_query_argument "arg1" secret2 - . add_query_argument "arg2" "plain value" - - HTTP.clear_response_cache - HTTP.fetch url1 - get_num_response_cache_entries . should_equal 1 - HTTP.fetch uri2 - get_num_response_cache_entries . should_equal 2 - - group_builder.specify "Should work with secrets in the headers" pending=pending_has_url <| Test.with_retries <| - cloud_setup.with_prepared_environment <| - secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" - secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" - cleanup = - secret1.delete - secret2.delete - Panic.with_finalizer cleanup <| - # Requests differ only in secrets in headers. - uri = URI.from 'https://httpbin.org/bytes/50' - headers1 = [Header.new "A-Header" secret1] - headers2 = [Header.new "A-Header" secret2] - - HTTP.clear_response_cache - HTTP.fetch headers=headers1 uri - get_num_response_cache_entries . should_equal 1 - HTTP.fetch headers=headers2 uri - get_num_response_cache_entries . should_equal 2 - - group_builder.specify "Should not be able to set the cache limits higher than the real limits" <| - Test.expect_panic IllegalArgumentException <| - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument - Test.expect_panic IllegalArgumentException <| - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument - Test.expect_panic IllegalArgumentException <| - usable_disk_space = Java_File.new enso_project.root_path . getUsableSpace - EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly usable_disk_space+1 . should_fail_with Illegal_Argument - - group_builder.specify "Cache limits should have defaults" <| - lru_cache = EnsoSecretHelper.getCache.getLRUCache - lru_cache.getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) - lru_cache.getTotalCacheLimit.percentage . should_equal 0.2 - - group_builder.specify "Should be able to set the max file size via environment variable" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - lru_cache = LRUCache.new - lru_cache.getMaxFileSize . should_equal (8 * 1024 * 1024) - - group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) - - group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable, and track changes to available disk space." <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| - Panic.with_finalizer reset_disk_space <| + d _ _ = + Panic.with_finalizer file_deleter <| + Data.download base_url_with_slash+"test_download?length=200" target_file + target_file.read.length . should_equal 200 + with_config 100 (TotalCacheLimit.Bytes.new 120) d + + ## + group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + + expect_counts [0, 0] <| + Data.post url_post (Request_Body.Text "hello world") + + group_builder.specify "HTTP request with a non-GET method should reject a cache_policy=Use_Cache argument" pending=pending_has_url <| Test.with_retries <| + request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty + HTTP.new.request request cache_policy=Cache_Policy.Use_Cache . should_fail_with Illegal_Argument + + group_builder.specify "HTTP request with a non-GET method should not reject a cache_policy=No_Cache argument" pending=pending_has_url <| Test.with_retries <| + request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty + HTTP.new.request request cache_policy=Cache_Policy.No_Cache . should_succeed + + group_builder.specify "Should be able to clear caches" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + Data.fetch url0 + get_num_response_cache_entries . should_equal 1 + HTTP.clear_response_cache + get_num_response_cache_entries . should_equal 0 + + group_builder.specify "Cache key should depend on the headers" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + expect_counts [0, 2] <| + Data.fetch url0 headers=headers0 + Data.fetch url0 headers=headers1 + Data.fetch url0 headers=headers1 + Data.fetch url0 headers=headers0 + Data.fetch url0 headers=headers0 + Data.fetch url0 headers=headers1 + + group_builder.specify "Cache key should not depend on header order" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + header0 = Header.new "Abc" "eef" + header1 = Header.new "Abc" "def" + header2 = Header.new "Ghi" "jkl" + orders = [[header0, header1, header2], [header1, header2, header0], [header2, header1, header0]] + responses = orders.map headers-> + Data.fetch url0 headers=headers . decode_as_text + get_num_response_cache_entries . should_equal 1 + responses.distinct.length . should_equal 1 + + ## Fetching the trigger uri causes stale entries to be removed, since the + uri is always different and so the caching and cleanup logic is run. + fake_now = Date_Time.now + trigger_uri_serial = Ref.new 0 + make_trigger_uri = + serial = trigger_uri_serial.get + trigger_uri_serial.modify (_ + 1) + base_url_with_slash+'test_download?max-age=10000&length=50&abc='+serial.to_text + set_time_and_get_count advance_secs = + EnsoSecretHelper.getCache.getCacheTestParameters.setNowOverrideTestOnly (fake_now + (Duration.new seconds=advance_secs)) + trigger_uri = make_trigger_uri + Data.fetch trigger_uri + get_num_response_cache_entries + fake_time_resetter = + EnsoSecretHelper.getCache.getCacheTestParameters.clearNowOverrideTestOnly + + group_builder.specify "The cache should expire stale entries" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + + set_time_and_get_count 0 # Initialize fake now. + + Data.fetch base_url_with_slash+'test_download?max-age=100&length=50' + Data.fetch base_url_with_slash+'test_download?max-age=200&length=50' + Data.fetch base_url_with_slash+'test_download?max-age=200&length=51' + Data.fetch base_url_with_slash+'test_download?max-age=300&length=50' + + Panic.with_finalizer fake_time_resetter <| + ## The count will increase by 1 each time, but decrease by the + number of entries removed + set_time_and_get_count 0 . should_equal 6 + set_time_and_get_count 90 . should_equal 7 + set_time_and_get_count 110 . should_equal 7 + set_time_and_get_count 190 . should_equal 8 + set_time_and_get_count 202 . should_equal 7 + set_time_and_get_count 292 . should_equal 8 + set_time_and_get_count 301 . should_equal 8 + + group_builder.specify "The cache should use the Age response header" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + + set_time_and_get_count 0 # Initialize fake now. + + Data.fetch base_url_with_slash+'test_download?max-age=100&age=50&length=50' # ttl 50 + Data.fetch base_url_with_slash+'test_download?max-age=100&age=30&length=50' # ttl 70 + Data.fetch base_url_with_slash+'test_download?max-age=120&age=50&length=50' # ttl 70 + Data.fetch base_url_with_slash+'test_download?max-age=70&&length=50' # ttl 70 + Data.fetch base_url_with_slash+'test_download?max-age=160&age=70&length=50' # ttl 90 + + Panic.with_finalizer fake_time_resetter <| + ## The count will increase by 1 each time, but decrease by the + number of entries removed + set_time_and_get_count 0 . should_equal 7 + set_time_and_get_count 40 . should_equal 8 + set_time_and_get_count 51 . should_equal 8 + set_time_and_get_count 68 . should_equal 9 + set_time_and_get_count 72 . should_equal 7 + set_time_and_get_count 88 . should_equal 8 + set_time_and_get_count 93 . should_equal 8 + + download size = + Data.fetch base_url_with_slash+'test_download?length='+size.to_text + + group_builder.specify "Will remove old cache files to keep the total cache size under the total cache size limit" pending=pending_has_url <| Test.with_retries <| + Panic.with_finalizer reset_size_limits <| + reset_size_limits + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 + + download 30 + download 50 + download 10 + get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] + download 20 + get_cache_file_sizes . should_equal_ignoring_order [10, 20, 50] + download 40 + get_cache_file_sizes . should_equal_ignoring_order [10, 20, 40] + download 35 + get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] + + group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| + Panic.with_finalizer reset_size_limits <| + reset_size_limits + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 + + download 30 + download 50 + download 10 + get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] + # Use 30 again so it's considered more recently used. + download 30 + get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] + download 20 + get_cache_file_sizes . should_equal_ignoring_order [10, 20, 30] + download 45 + get_cache_file_sizes . should_equal_ignoring_order [20, 30, 45] + + group_builder.specify "Will not cache a file with a content length greater than the single file limit" pending=pending_has_url <| Test.with_retries <| + Panic.with_finalizer reset_size_limits <| + reset_size_limits + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 + download 110 . should_fail_with (Response_Too_Large.Error 100) + + group_builder.specify "Will not cache a file without a content length, but which is greater than the single file limit" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + Panic.with_finalizer reset_size_limits <| + reset_size_limits + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 + url = base_url_with_slash+'test_download?omit-content-length=1&length=110' + Data.fetch url . should_fail_with (Response_Too_Large.Error 100) + + group_builder.specify "Should not cache if the request fails" pending=pending_has_url <| Test.with_retries <| + HTTP.clear_response_cache + + HTTP.fetch url0 + get_num_response_cache_entries . should_equal 1 + HTTP.fetch base_url_with_slash+'crash' + get_num_response_cache_entries . should_equal 1 + HTTP.fetch base_url_with_slash+'nonexistent_endpoint' + get_num_response_cache_entries . should_equal 1 + + cloud_setup = Cloud_Tests_Setup.prepare + + group_builder.specify "Should work with secrets in the URI" pending=pending_has_url <| Test.with_retries <| + cloud_setup.with_prepared_environment <| + secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" + secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" + cleanup = + secret1.delete + secret2.delete + Panic.with_finalizer cleanup <| + # Requests differ only in secrets in URI. + url1 = URI.from 'https://httpbin.org/bytes/50' + . add_query_argument "arg1" secret1 + . add_query_argument "arg2" "plain value" + uri2 = URI.from 'https://httpbin.org/bytes/50' + . add_query_argument "arg1" secret2 + . add_query_argument "arg2" "plain value" + + HTTP.clear_response_cache + HTTP.fetch url1 + get_num_response_cache_entries . should_equal 1 + HTTP.fetch uri2 + get_num_response_cache_entries . should_equal 2 + + group_builder.specify "Should work with secrets in the headers" pending=pending_has_url <| Test.with_retries <| + cloud_setup.with_prepared_environment <| + secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" + secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" + cleanup = + secret1.delete + secret2.delete + Panic.with_finalizer cleanup <| + # Requests differ only in secrets in headers. + uri = URI.from 'https://httpbin.org/bytes/50' + headers1 = [Header.new "A-Header" secret1] + headers2 = [Header.new "A-Header" secret2] + + HTTP.clear_response_cache + HTTP.fetch headers=headers1 uri + get_num_response_cache_entries . should_equal 1 + HTTP.fetch headers=headers2 uri + get_num_response_cache_entries . should_equal 2 + + group_builder.specify "Should not be able to set the cache limits higher than the real limits" <| + Test.expect_panic IllegalArgumentException <| + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument + Test.expect_panic IllegalArgumentException <| + EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument + Test.expect_panic IllegalArgumentException <| + usable_disk_space = Java_File.new enso_project.root_path . getUsableSpace + EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly usable_disk_space+1 . should_fail_with Illegal_Argument + + group_builder.specify "Cache limits should have defaults" <| + lru_cache = EnsoSecretHelper.getCache.getLRUCache + lru_cache.getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) + lru_cache.getTotalCacheLimit.percentage . should_equal 0.2 + + group_builder.specify "Should be able to set the max file size via environment variable" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| lru_cache = LRUCache.new - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 300 - lru_cache.getMaxTotalCacheSize . should_equal 30 - lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 400 - lru_cache.getMaxTotalCacheSize . should_equal 40 - lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 - - group_builder.specify "Total cache size should not go over the hard-coded maximum" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) - - group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| - Test.expect_panic NumberFormatException LRUCache.new - - group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| - Test.expect_panic IllegalArgumentException LRUCache.new - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| - Test.expect_panic IllegalArgumentException LRUCache.new + lru_cache.getMaxFileSize . should_equal (8 * 1024 * 1024) + + group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| + lru_cache = LRUCache.new + lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) + + group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable, and track changes to available disk space." <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| + Panic.with_finalizer reset_disk_space <| + lru_cache = LRUCache.new + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 300 + lru_cache.getMaxTotalCacheSize . should_equal 30 + lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 + lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 400 + lru_cache.getMaxTotalCacheSize . should_equal 40 + lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 + + group_builder.specify "Total cache size should not go over the hard-coded maximum" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| + lru_cache = LRUCache.new + lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) + + group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| + Test.expect_panic NumberFormatException LRUCache.new + + group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| + Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| + Test.expect_panic IllegalArgumentException LRUCache.new From a1be3e6a164b4c722bc17d09cd3faa5a61a7b40a Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 14:37:22 -0600 Subject: [PATCH 19/40] 17 --- test/Table_Tests/src/IO/Fetch_Spec.enso | 44 ++++++++++++++----------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 26bfd5e19cf3..30143b67c9f5 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -149,6 +149,13 @@ add_specs suite_builder = lru_cache = LRUCache.new lru_cache_settings now_getter disk_space_getter with_lru_cache lru_cache (action now_getter disk_space_getter) + # Craetes a new cache each time, then resets it at the end + with_mocks ~action = + now_getter = NowGetter.new + disk_space_getter = DiskSpaceGetter.new + lru_cache = LRUCache.new + with_lru_cache lru_cache (action now_getter disk_space_getter) + # Craetes a new cache each time, then resets it at the end with_default_cache ~action = lru_cache = LRUCache.new @@ -252,30 +259,28 @@ add_specs suite_builder = target_file.read.length . should_equal 200 with_config 100 (TotalCacheLimit.Bytes.new 120) d - ## - group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - + group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| + with_default_cache <| expect_counts [0, 0] <| Data.post url_post (Request_Body.Text "hello world") - group_builder.specify "HTTP request with a non-GET method should reject a cache_policy=Use_Cache argument" pending=pending_has_url <| Test.with_retries <| - request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty - HTTP.new.request request cache_policy=Cache_Policy.Use_Cache . should_fail_with Illegal_Argument + group_builder.specify "HTTP request with a non-GET method should reject a cache_policy=Use_Cache argument" pending=pending_has_url <| Test.with_retries <| + request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty + HTTP.new.request request cache_policy=Cache_Policy.Use_Cache . should_fail_with Illegal_Argument - group_builder.specify "HTTP request with a non-GET method should not reject a cache_policy=No_Cache argument" pending=pending_has_url <| Test.with_retries <| - request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty - HTTP.new.request request cache_policy=Cache_Policy.No_Cache . should_succeed + group_builder.specify "HTTP request with a non-GET method should not reject a cache_policy=No_Cache argument" pending=pending_has_url <| Test.with_retries <| + request = Request.new HTTP_Method.Post url_post [] Request_Body.Empty + HTTP.new.request request cache_policy=Cache_Policy.No_Cache . should_succeed - group_builder.specify "Should be able to clear caches" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache + group_builder.specify "Should be able to clear caches" pending=pending_has_url <| Test.with_retries <| + with_default_cache <| Data.fetch url0 get_num_response_cache_entries . should_equal 1 - HTTP.clear_response_cache + with_default_cache <| get_num_response_cache_entries . should_equal 0 - group_builder.specify "Cache key should depend on the headers" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache + group_builder.specify "Cache key should depend on the headers" pending=pending_has_url <| Test.with_retries <| + with_default_cache <| expect_counts [0, 2] <| Data.fetch url0 headers=headers0 Data.fetch url0 headers=headers1 @@ -284,8 +289,8 @@ add_specs suite_builder = Data.fetch url0 headers=headers0 Data.fetch url0 headers=headers1 - group_builder.specify "Cache key should not depend on header order" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache + group_builder.specify "Cache key should not depend on header order" pending=pending_has_url <| Test.with_retries <| + with_default_cache <| header0 = Header.new "Abc" "eef" header1 = Header.new "Abc" "def" header2 = Header.new "Ghi" "jkl" @@ -295,8 +300,9 @@ add_specs suite_builder = get_num_response_cache_entries . should_equal 1 responses.distinct.length . should_equal 1 - ## Fetching the trigger uri causes stale entries to be removed, since the - uri is always different and so the caching and cleanup logic is run. + ## Fetching the trigger uri causes stale entries to be removed, since the + uri is always different and so the caching and cleanup logic is run. + ## fake_now = Date_Time.now trigger_uri_serial = Ref.new 0 make_trigger_uri = From 31a8f02479c18f46b55ab27f6e334043de734ce2 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 14:38:15 -0600 Subject: [PATCH 20/40] double lambda --- test/Table_Tests/src/IO/Fetch_Spec.enso | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 30143b67c9f5..d00272fc7afe 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -253,11 +253,10 @@ add_specs suite_builder = group_builder.specify "Data.download is not affected by caching limits" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" file_deleter = target_file.delete_if_exists - d _ _ = + with_config 100 (TotalCacheLimit.Bytes.new 120) _-> _-> Panic.with_finalizer file_deleter <| Data.download base_url_with_slash+"test_download?length=200" target_file target_file.read.length . should_equal 200 - with_config 100 (TotalCacheLimit.Bytes.new 120) d group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| with_default_cache <| From 4a87a2591448815c5cc61acfa7e6e5027a53546a Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 15:04:51 -0600 Subject: [PATCH 21/40] fix now --- .../java/org/enso/base/cache/NowGetter.java | 15 ++++- test/Table_Tests/src/IO/Fetch_Spec.enso | 60 +++++++++---------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index 8c53b90052bb..876bc5df5c64 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -3,7 +3,16 @@ import java.time.ZonedDateTime; public class NowGetter extends Mockable { - public NowGetter() { - super(() -> ZonedDateTime.now()); - } + public NowGetter() { + super(() -> ZonedDateTime.now()); + } + + /** + * This is necessary because a direct call to the superclass does not convert + * Value to ZonedDateTime. + */ + @Override + public void mocked(ZonedDateTime dt) { + super.mocked(dt); + } } diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index d00272fc7afe..7f4aec7dff75 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -153,7 +153,7 @@ add_specs suite_builder = with_mocks ~action = now_getter = NowGetter.new disk_space_getter = DiskSpaceGetter.new - lru_cache = LRUCache.new + lru_cache = LRUCache.new LRUCacheSettings.getDefault now_getter disk_space_getter with_lru_cache lru_cache (action now_getter disk_space_getter) # Craetes a new cache each time, then resets it at the end @@ -301,42 +301,42 @@ add_specs suite_builder = ## Fetching the trigger uri causes stale entries to be removed, since the uri is always different and so the caching and cleanup logic is run. - ## - fake_now = Date_Time.now - trigger_uri_serial = Ref.new 0 - make_trigger_uri = - serial = trigger_uri_serial.get - trigger_uri_serial.modify (_ + 1) - base_url_with_slash+'test_download?max-age=10000&length=50&abc='+serial.to_text - set_time_and_get_count advance_secs = - EnsoSecretHelper.getCache.getCacheTestParameters.setNowOverrideTestOnly (fake_now + (Duration.new seconds=advance_secs)) - trigger_uri = make_trigger_uri - Data.fetch trigger_uri - get_num_response_cache_entries - fake_time_resetter = - EnsoSecretHelper.getCache.getCacheTestParameters.clearNowOverrideTestOnly - - group_builder.specify "The cache should expire stale entries" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - set_time_and_get_count 0 # Initialize fake now. + fake_now = Date_Time.now + trigger_uri_serial = Ref.new 0 + make_trigger_uri = + serial = trigger_uri_serial.get + trigger_uri_serial.modify (_ + 1) + base_url_with_slash+'test_download?max-age=10000&length=50&abc='+serial.to_text + set_time_and_get_count now_mock advance_secs = + now_mock.lala (fake_now + (Duration.new seconds=advance_secs)) + now_mock.mocked (fake_now + (Duration.new seconds=advance_secs)) + trigger_uri = make_trigger_uri + Data.fetch trigger_uri + get_num_response_cache_entries + fake_time_resetter = + EnsoSecretHelper.getCache.getCacheTestParameters.clearNowOverrideTestOnly + + group_builder.specify "The cache should expire stale entries" pending=pending_has_url <| Test.with_retries <| + with_mocks now-> _-> + + set_time_and_get_count now 0 # Initialize fake now. Data.fetch base_url_with_slash+'test_download?max-age=100&length=50' Data.fetch base_url_with_slash+'test_download?max-age=200&length=50' Data.fetch base_url_with_slash+'test_download?max-age=200&length=51' Data.fetch base_url_with_slash+'test_download?max-age=300&length=50' - Panic.with_finalizer fake_time_resetter <| - ## The count will increase by 1 each time, but decrease by the - number of entries removed - set_time_and_get_count 0 . should_equal 6 - set_time_and_get_count 90 . should_equal 7 - set_time_and_get_count 110 . should_equal 7 - set_time_and_get_count 190 . should_equal 8 - set_time_and_get_count 202 . should_equal 7 - set_time_and_get_count 292 . should_equal 8 - set_time_and_get_count 301 . should_equal 8 + ## The count will increase by 1 each time, but decrease by the + number of entries removed + set_time_and_get_count now 0 . should_equal 6 + set_time_and_get_count now 90 . should_equal 7 + set_time_and_get_count now 110 . should_equal 7 + set_time_and_get_count now 190 . should_equal 8 + set_time_and_get_count now 202 . should_equal 7 + set_time_and_get_count now 292 . should_equal 8 + set_time_and_get_count now 301 . should_equal 8 + ## group_builder.specify "The cache should use the Age response header" pending=pending_has_url <| Test.with_retries <| HTTP.clear_response_cache From 0e44c66f5d88378c44244b0861d7dac55ec76c01 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 15:36:39 -0600 Subject: [PATCH 22/40] more --- .../java/org/enso/base/cache/LRUCache.java | 5 + test/Table_Tests/src/IO/Fetch_Spec.enso | 290 ++++++++---------- 2 files changed, 133 insertions(+), 162 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index caba59003bc6..83f9bee1ef3d 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -282,6 +282,11 @@ public List getFileSizesTestOnly() { cache.values().stream().map(CacheEntry::size).collect(Collectors.toList())); } + /** Public for testing. */ + public LRUCacheSettings getSettings() { + return settings; + } + private record CacheEntry(File responseData, M metadata, long size, ZonedDateTime expiry) {} /** diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 7f4aec7dff75..74c160e58724 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -308,17 +308,13 @@ add_specs suite_builder = trigger_uri_serial.modify (_ + 1) base_url_with_slash+'test_download?max-age=10000&length=50&abc='+serial.to_text set_time_and_get_count now_mock advance_secs = - now_mock.lala (fake_now + (Duration.new seconds=advance_secs)) now_mock.mocked (fake_now + (Duration.new seconds=advance_secs)) trigger_uri = make_trigger_uri Data.fetch trigger_uri get_num_response_cache_entries - fake_time_resetter = - EnsoSecretHelper.getCache.getCacheTestParameters.clearNowOverrideTestOnly group_builder.specify "The cache should expire stale entries" pending=pending_has_url <| Test.with_retries <| with_mocks now-> _-> - set_time_and_get_count now 0 # Initialize fake now. Data.fetch base_url_with_slash+'test_download?max-age=100&length=50' @@ -336,11 +332,9 @@ add_specs suite_builder = set_time_and_get_count now 292 . should_equal 8 set_time_and_get_count now 301 . should_equal 8 - ## - group_builder.specify "The cache should use the Age response header" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - - set_time_and_get_count 0 # Initialize fake now. + group_builder.specify "The cache should use the Age response header" pending=pending_has_url <| Test.with_retries <| + with_mocks now-> _-> + set_time_and_get_count now 0 # Initialize fake now. Data.fetch base_url_with_slash+'test_download?max-age=100&age=50&length=50' # ttl 50 Data.fetch base_url_with_slash+'test_download?max-age=100&age=30&length=50' # ttl 70 @@ -348,70 +342,57 @@ add_specs suite_builder = Data.fetch base_url_with_slash+'test_download?max-age=70&&length=50' # ttl 70 Data.fetch base_url_with_slash+'test_download?max-age=160&age=70&length=50' # ttl 90 - Panic.with_finalizer fake_time_resetter <| - ## The count will increase by 1 each time, but decrease by the - number of entries removed - set_time_and_get_count 0 . should_equal 7 - set_time_and_get_count 40 . should_equal 8 - set_time_and_get_count 51 . should_equal 8 - set_time_and_get_count 68 . should_equal 9 - set_time_and_get_count 72 . should_equal 7 - set_time_and_get_count 88 . should_equal 8 - set_time_and_get_count 93 . should_equal 8 - - download size = - Data.fetch base_url_with_slash+'test_download?length='+size.to_text - - group_builder.specify "Will remove old cache files to keep the total cache size under the total cache size limit" pending=pending_has_url <| Test.with_retries <| - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 - - download 30 - download 50 - download 10 - get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - download 20 - get_cache_file_sizes . should_equal_ignoring_order [10, 20, 50] - download 40 - get_cache_file_sizes . should_equal_ignoring_order [10, 20, 40] - download 35 - get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] - - group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly 100 - - download 30 - download 50 - download 10 - get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - # Use 30 again so it's considered more recently used. - download 30 - get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - download 20 - get_cache_file_sizes . should_equal_ignoring_order [10, 20, 30] - download 45 - get_cache_file_sizes . should_equal_ignoring_order [20, 30, 45] - - group_builder.specify "Will not cache a file with a content length greater than the single file limit" pending=pending_has_url <| Test.with_retries <| - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 - download 110 . should_fail_with (Response_Too_Large.Error 100) - - group_builder.specify "Will not cache a file without a content length, but which is greater than the single file limit" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - Panic.with_finalizer reset_size_limits <| - reset_size_limits - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly 100 - url = base_url_with_slash+'test_download?omit-content-length=1&length=110' - Data.fetch url . should_fail_with (Response_Too_Large.Error 100) - - group_builder.specify "Should not cache if the request fails" pending=pending_has_url <| Test.with_retries <| - HTTP.clear_response_cache - + ## The count will increase by 1 each time, but decrease by the + number of entries removed + set_time_and_get_count now 0 . should_equal 7 + set_time_and_get_count now 40 . should_equal 8 + set_time_and_get_count now 51 . should_equal 8 + set_time_and_get_count now 68 . should_equal 9 + set_time_and_get_count now 72 . should_equal 7 + set_time_and_get_count now 88 . should_equal 8 + set_time_and_get_count now 93 . should_equal 8 + + download size = + Data.fetch base_url_with_slash+'test_download?length='+size.to_text + + group_builder.specify "Will remove old cache files to keep the total cache size under the total cache size limit" pending=pending_has_url <| Test.with_retries <| + with_config 1000 (TotalCacheLimit.Bytes.new 100) _-> _-> + download 30 + download 50 + download 10 + get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] + download 20 + get_cache_file_sizes . should_equal_ignoring_order [10, 20, 50] + download 40 + get_cache_file_sizes . should_equal_ignoring_order [10, 20, 40] + download 35 + get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] + + group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| + with_config 1000 (TotalCacheLimit.Bytes.new 100) _-> _-> + download 30 + download 50 + download 10 + get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] + # Use 30 again so it's considered more recently used. + download 30 + get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] + download 20 + get_cache_file_sizes . should_equal_ignoring_order [10, 20, 30] + download 45 + get_cache_file_sizes . should_equal_ignoring_order [20, 30, 45] + + group_builder.specify "Will not cache a file with a content length greater than the single file limit" pending=pending_has_url <| Test.with_retries <| + with_config 100 (TotalCacheLimit.Bytes.new 1000) _-> _-> + download 110 . should_fail_with (Response_Too_Large.Error 100) + + group_builder.specify "Will not cache a file without a content length, but which is greater than the single file limit" pending=pending_has_url <| Test.with_retries <| + with_config 100 (TotalCacheLimit.Bytes.new 1000) _-> _-> + url = base_url_with_slash+'test_download?omit-content-length=1&length=110' + Data.fetch url . should_fail_with (Response_Too_Large.Error 100) + + group_builder.specify "Should not cache if the request fails" pending=pending_has_url <| Test.with_retries <| + with_default_cache <| HTTP.fetch url0 get_num_response_cache_entries . should_equal 1 HTTP.fetch base_url_with_slash+'crash' @@ -419,95 +400,80 @@ add_specs suite_builder = HTTP.fetch base_url_with_slash+'nonexistent_endpoint' get_num_response_cache_entries . should_equal 1 - cloud_setup = Cloud_Tests_Setup.prepare - - group_builder.specify "Should work with secrets in the URI" pending=pending_has_url <| Test.with_retries <| - cloud_setup.with_prepared_environment <| - secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" - secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" - cleanup = - secret1.delete - secret2.delete - Panic.with_finalizer cleanup <| - # Requests differ only in secrets in URI. - url1 = URI.from 'https://httpbin.org/bytes/50' - . add_query_argument "arg1" secret1 - . add_query_argument "arg2" "plain value" - uri2 = URI.from 'https://httpbin.org/bytes/50' - . add_query_argument "arg1" secret2 - . add_query_argument "arg2" "plain value" - - HTTP.clear_response_cache - HTTP.fetch url1 - get_num_response_cache_entries . should_equal 1 - HTTP.fetch uri2 - get_num_response_cache_entries . should_equal 2 - - group_builder.specify "Should work with secrets in the headers" pending=pending_has_url <| Test.with_retries <| - cloud_setup.with_prepared_environment <| - secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" - secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" - cleanup = - secret1.delete - secret2.delete - Panic.with_finalizer cleanup <| - # Requests differ only in secrets in headers. - uri = URI.from 'https://httpbin.org/bytes/50' - headers1 = [Header.new "A-Header" secret1] - headers2 = [Header.new "A-Header" secret2] - - HTTP.clear_response_cache - HTTP.fetch headers=headers1 uri - get_num_response_cache_entries . should_equal 1 - HTTP.fetch headers=headers2 uri - get_num_response_cache_entries . should_equal 2 - - group_builder.specify "Should not be able to set the cache limits higher than the real limits" <| - Test.expect_panic IllegalArgumentException <| - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxFileSizeOverrideTestOnly (2 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument - Test.expect_panic IllegalArgumentException <| - EnsoSecretHelper.getCache.getCacheTestParameters.setMaxTotalCacheSizeOverrideTestOnly (20 * 1024 * 1024 * 1024 + 1) . should_fail_with Illegal_Argument - Test.expect_panic IllegalArgumentException <| - usable_disk_space = Java_File.new enso_project.root_path . getUsableSpace - EnsoSecretHelper.getCache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly usable_disk_space+1 . should_fail_with Illegal_Argument - - group_builder.specify "Cache limits should have defaults" <| - lru_cache = EnsoSecretHelper.getCache.getLRUCache - lru_cache.getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) - lru_cache.getTotalCacheLimit.percentage . should_equal 0.2 - - group_builder.specify "Should be able to set the max file size via environment variable" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - lru_cache = LRUCache.new - lru_cache.getMaxFileSize . should_equal (8 * 1024 * 1024) - - group_builder.specify "Should be able to set the max total cache size in bytes via environment variable" <| + cloud_setup = Cloud_Tests_Setup.prepare + + group_builder.specify "Should work with secrets in the URI" pending=pending_has_url <| Test.with_retries <| + cloud_setup.with_prepared_environment <| + secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" + secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" + cleanup = + secret1.delete + secret2.delete + with_default_cache <| Panic.with_finalizer cleanup <| + # Requests differ only in secrets in URI. + url1 = URI.from 'https://httpbin.org/bytes/50' + . add_query_argument "arg1" secret1 + . add_query_argument "arg2" "plain value" + uri2 = URI.from 'https://httpbin.org/bytes/50' + . add_query_argument "arg1" secret2 + . add_query_argument "arg2" "plain value" + + HTTP.fetch url1 + get_num_response_cache_entries . should_equal 1 + HTTP.fetch uri2 + get_num_response_cache_entries . should_equal 2 + + group_builder.specify "Should work with secrets in the headers" pending=pending_has_url <| Test.with_retries <| + cloud_setup.with_prepared_environment <| + secret1 = Enso_Secret.create "http-cache-secret-1-"+Random.uuid "My Value" + secret2 = Enso_Secret.create "http-cache-secret-2-"+Random.uuid "Some Value" + cleanup = + secret1.delete + secret2.delete + with_default_cache <| Panic.with_finalizer cleanup <| + # Requests differ only in secrets in headers. + uri = URI.from 'https://httpbin.org/bytes/50' + headers1 = [Header.new "A-Header" secret1] + headers2 = [Header.new "A-Header" secret2] + + HTTP.fetch headers=headers1 uri + get_num_response_cache_entries . should_equal 1 + HTTP.fetch headers=headers2 uri + get_num_response_cache_entries . should_equal 2 + + group_builder.specify "Cache limits should have defaults" <| + lru_cache = LRUCache.new + lru_cache.getSettings.getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) + lru_cache.getSettings.getTotalCacheLimit.percentage . should_equal 0.2 + + group_builder.specify "Should be able to set the max file size and total cache size (in bytes) via environment variable" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) + with_default_cache <| + EnsoSecretHelper.getCache.getLRUCache.getSettings.getMaxFileSize . should_equal (8 * 1024 * 1024) + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) - group_builder.specify "Should be able to set the max total cache size as a percentage via environment variable, and track changes to available disk space." <| + group_builder.specify "Should be able to set the max file size and total cache size (as a percentage) via environment variable, and track changes to available disk space" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| - Panic.with_finalizer reset_disk_space <| - lru_cache = LRUCache.new - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 300 - lru_cache.getMaxTotalCacheSize . should_equal 30 - lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 - lru_cache.getCacheTestParameters.setUsableDiskSpaceOverrideTestOnly 400 - lru_cache.getMaxTotalCacheSize . should_equal 40 - lru_cache.getTotalCacheLimit.percentage . should_equal 0.1 - - group_builder.specify "Total cache size should not go over the hard-coded maximum" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) - - group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| - Test.expect_panic NumberFormatException LRUCache.new - - group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| - Test.expect_panic IllegalArgumentException LRUCache.new - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| - Test.expect_panic IllegalArgumentException LRUCache.new + with_mocks _-> disk_space-> + EnsoSecretHelper.getCache.getLRUCache.getSettings.getMaxFileSize . should_equal (8 * 1024 * 1024) + disk_space.mocked 300 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) + disk_space.mocked 400 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal (40 * 1024 * 1024) + + group_builder.specify "Total cache size should not go over the hard-coded maximum" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| + lru_cache = LRUCache.new + lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) + + group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| + Test.expect_panic NumberFormatException LRUCache.new + + group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| + Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| + Test.expect_panic IllegalArgumentException LRUCache.new From edc18cb1c0f3a2484cda814fa3ca8fe0e609092f Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 15:45:43 -0600 Subject: [PATCH 23/40] green --- test/Table_Tests/src/IO/Fetch_Spec.enso | 26 ++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 74c160e58724..b807a5c2a263 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -459,21 +459,25 @@ add_specs suite_builder = with_mocks _-> disk_space-> EnsoSecretHelper.getCache.getLRUCache.getSettings.getMaxFileSize . should_equal (8 * 1024 * 1024) disk_space.mocked 300 - EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 30 disk_space.mocked 400 - EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal (40 * 1024 * 1024) + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 40 group_builder.specify "Total cache size should not go over the hard-coded maximum" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| + lru_cache = LRUCache.new + lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| - Test.expect_panic NumberFormatException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| + Test.expect_panic NumberFormatException LRUCache.new group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| - Test.expect_panic IllegalArgumentException LRUCache.new - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| - Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| + Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| + Test.expect_panic IllegalArgumentException LRUCache.new From 3b079f92d0b1b284de43aafd2c419c6e1cf246a3 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 15:52:13 -0600 Subject: [PATCH 24/40] do not have to set both env vars --- .../org/enso/base/cache/LRUCacheSettings.java | 12 +++------- test/Table_Tests/src/IO/Fetch_Spec.enso | 22 ++++++++----------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java index 2914dbcbed51..31308106f7d5 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -35,19 +35,13 @@ public LRUCacheSettings(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) this.totalCacheLimit = totalCacheLimit; } - public LRUCacheSettings(String maxFileSizeSpec, String totalCacheLimitSpec) { - this(parseMaxFileSize(maxFileSizeSpec), parseTotalCacheLimit(totalCacheLimitSpec)); - } - /** Uses defaults if the vars are not set. */ public static LRUCacheSettings getDefault() { String maxFileSizeSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); String totalCacheLimitSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); - if (maxFileSizeSpec != null && totalCacheLimitSpec != null) { - return new LRUCacheSettings(maxFileSizeSpec, totalCacheLimitSpec); - } else { - return new LRUCacheSettings(DEFAULT_MAX_FILE_SIZE, new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE)); - } + var maxFileSize = maxFileSizeSpec != null ? parseMaxFileSize(maxFileSizeSpec) : DEFAULT_MAX_FILE_SIZE; + var totalCacheLimit = totalCacheLimitSpec != null ? parseTotalCacheLimit(totalCacheLimitSpec) : new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + return new LRUCacheSettings(maxFileSize, totalCacheLimit); } public long getMaxFileSize() { diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index b807a5c2a263..d04e94d16711 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -464,20 +464,16 @@ add_specs suite_builder = EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 40 group_builder.specify "Total cache size should not go over the hard-coded maximum" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| + lru_cache = LRUCache.new + lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| - Test.expect_panic NumberFormatException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| + Test.expect_panic NumberFormatException LRUCache.new group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| - Test.expect_panic IllegalArgumentException LRUCache.new - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| - Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| + Test.expect_panic IllegalArgumentException LRUCache.new + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| + Test.expect_panic IllegalArgumentException LRUCache.new From 3d2fcf9a07a4497eec6f4015e407b85e6c998d51 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 15:58:19 -0600 Subject: [PATCH 25/40] download not limited check checks that fetch throws --- test/Table_Tests/src/IO/Fetch_Spec.enso | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index d04e94d16711..186a42f73e52 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -255,8 +255,10 @@ add_specs suite_builder = file_deleter = target_file.delete_if_exists with_config 100 (TotalCacheLimit.Bytes.new 120) _-> _-> Panic.with_finalizer file_deleter <| - Data.download base_url_with_slash+"test_download?length=200" target_file + url = base_url_with_slash+"test_download?length=200" + Data.download url target_file target_file.read.length . should_equal 200 + Data.fetch url . should_fail_with (Response_Too_Large.Error 100) group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| with_default_cache <| From 141d6a23399a25e5f24e3c134bda2a8d8729a635 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 16:06:06 -0600 Subject: [PATCH 26/40] file_deleter, fetch_n --- test/Table_Tests/src/IO/Fetch_Spec.enso | 53 ++++++++++++------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 186a42f73e52..30f5bf6b7dc7 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -114,6 +114,12 @@ add_specs suite_builder = r4.should_equal (Table.from_rows ["Column 1"] [["A,B"], ["1,x"], ["3,y"]]) suite_builder.group "Response caching" pending=pending_has_url group_builder-> + with_temp_file file ~action = + deleter = file.delete_if_exists + Panic.with_finalizer deleter <| + deleter + action + get_lru_cache = EnsoSecretHelper.getCache.getLRUCache @@ -241,24 +247,17 @@ add_specs suite_builder = group_builder.specify "Should not cache Data.download" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" - - with_default_cache <| - target_file.delete_if_exists - + with_temp_file target_file <| with_default_cache <| Data.download url0 target_file get_num_response_cache_entries . should_equal 0 - target_file.delete_if_exists - group_builder.specify "Data.download is not affected by caching limits" pending=pending_has_url <| Test.with_retries <| target_file = enso_project.data / "transient" / "my_download0.txt" - file_deleter = target_file.delete_if_exists - with_config 100 (TotalCacheLimit.Bytes.new 120) _-> _-> - Panic.with_finalizer file_deleter <| - url = base_url_with_slash+"test_download?length=200" - Data.download url target_file - target_file.read.length . should_equal 200 - Data.fetch url . should_fail_with (Response_Too_Large.Error 100) + with_temp_file target_file <| with_config 100 (TotalCacheLimit.Bytes.new 120) _-> _-> + url = base_url_with_slash+"test_download?length=200" + Data.download url target_file + target_file.read.length . should_equal 200 + Data.fetch url . should_fail_with (Response_Too_Large.Error 100) group_builder.specify "Should not cache for methods other than GET" pending=pending_has_url <| Test.with_retries <| with_default_cache <| @@ -354,39 +353,39 @@ add_specs suite_builder = set_time_and_get_count now 88 . should_equal 8 set_time_and_get_count now 93 . should_equal 8 - download size = + fetch_n size = Data.fetch base_url_with_slash+'test_download?length='+size.to_text group_builder.specify "Will remove old cache files to keep the total cache size under the total cache size limit" pending=pending_has_url <| Test.with_retries <| with_config 1000 (TotalCacheLimit.Bytes.new 100) _-> _-> - download 30 - download 50 - download 10 + fetch_n 30 + fetch_n 50 + fetch_n 10 get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - download 20 + fetch_n 20 get_cache_file_sizes . should_equal_ignoring_order [10, 20, 50] - download 40 + fetch_n 40 get_cache_file_sizes . should_equal_ignoring_order [10, 20, 40] - download 35 + fetch_n 35 get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| with_config 1000 (TotalCacheLimit.Bytes.new 100) _-> _-> - download 30 - download 50 - download 10 + fetch_n 30 + fetch_n 50 + fetch_n 10 get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] # Use 30 again so it's considered more recently used. - download 30 + fetch_n 30 get_cache_file_sizes . should_equal_ignoring_order [10, 30, 50] - download 20 + fetch_n 20 get_cache_file_sizes . should_equal_ignoring_order [10, 20, 30] - download 45 + fetch_n 45 get_cache_file_sizes . should_equal_ignoring_order [20, 30, 45] group_builder.specify "Will not cache a file with a content length greater than the single file limit" pending=pending_has_url <| Test.with_retries <| with_config 100 (TotalCacheLimit.Bytes.new 1000) _-> _-> - download 110 . should_fail_with (Response_Too_Large.Error 100) + fetch_n 110 . should_fail_with (Response_Too_Large.Error 100) group_builder.specify "Will not cache a file without a content length, but which is greater than the single file limit" pending=pending_has_url <| Test.with_retries <| with_config 100 (TotalCacheLimit.Bytes.new 1000) _-> _-> From 46634f54af95d0d0ec9acf6c2c68da39c174c822 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 16:11:07 -0600 Subject: [PATCH 27/40] deleter fix --- test/Table_Tests/src/IO/Fetch_Spec.enso | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 30f5bf6b7dc7..0dd4db5e0373 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -115,7 +115,8 @@ add_specs suite_builder = suite_builder.group "Response caching" pending=pending_has_url group_builder-> with_temp_file file ~action = - deleter = file.delete_if_exists + deleter = + file.delete_if_exists Panic.with_finalizer deleter <| deleter action From 8500533e0b24d8d241a5f9f5e4f925835006fe80 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 16:13:16 -0600 Subject: [PATCH 28/40] wip --- std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java | 3 ++- test/Table_Tests/src/IO/Fetch_Spec.enso | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 83f9bee1ef3d..c753458ec557 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -277,7 +277,8 @@ public int getNumEntries() { return cache.size(); } - public List getFileSizesTestOnly() { + /** Public for testing. */ + public List getFileSizes() { return new ArrayList<>( cache.values().stream().map(CacheEntry::size).collect(Collectors.toList())); } diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 0dd4db5e0373..d01bfac93f1b 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -139,7 +139,7 @@ add_specs suite_builder = get_cache_file_sizes : Vector Integer get_cache_file_sizes -> Vector Integer = - Vector.from_polyglot_array EnsoSecretHelper.getCache.getLRUCache.getFileSizesTestOnly . sort Sort_Direction.Ascending + Vector.from_polyglot_array EnsoSecretHelper.getCache.getLRUCache.getFileSizes . sort Sort_Direction.Ascending # Craetes a new cache each time, then resets it at the end with_lru_cache lru_cache ~action = From 6d420a24d72c001abeec47fdb1a3f8f0cdc2b998 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Fri, 8 Nov 2024 16:14:23 -0600 Subject: [PATCH 29/40] fmt --- .../org/enso/base/CurrentEnsoProject.java | 7 ++- .../org/enso/base/cache/DiskSpaceGetter.java | 2 +- .../java/org/enso/base/cache/LRUCache.java | 41 +++++++-------- .../org/enso/base/cache/LRUCacheSettings.java | 31 ++++++----- .../java/org/enso/base/cache/NowGetter.java | 6 +-- .../org/enso/base/cache/TotalCacheLimit.java | 52 ++++++++++--------- .../enso_cloud/EnsoHTTPResponseCache.java | 5 +- 7 files changed, 75 insertions(+), 69 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java b/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java index dccbdf27497e..5f387e21477b 100644 --- a/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java +++ b/std-bits/base/src/main/java/org/enso/base/CurrentEnsoProject.java @@ -22,14 +22,17 @@ public static CurrentEnsoProject get() { if (!isCached) { Value ensoProject = EnsoMeta.callStaticModuleMethod("Standard.Base.Meta.Enso_Project", "enso_project"); - if (ensoProject.hasMember("name") && ensoProject.hasMember("namespace") && ensoProject.hasMember("root_path")) { + if (ensoProject.hasMember("name") + && ensoProject.hasMember("namespace") + && ensoProject.hasMember("root_path")) { Value namespace = ensoProject.invokeMember("namespace"); Value name = ensoProject.invokeMember("name"); Value rootPath = ensoProject.invokeMember("root_path"); if (namespace == null || name == null || rootPath == null) { cached = null; } else { - cached = new CurrentEnsoProject(name.asString(), namespace.asString(), rootPath.asString()); + cached = + new CurrentEnsoProject(name.asString(), namespace.asString(), rootPath.asString()); } } else { cached = null; diff --git a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java index 1e2e77107c7e..758d7aa96d59 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java @@ -5,7 +5,7 @@ public class DiskSpaceGetter extends Mockable { public DiskSpaceGetter() { - super(() -> getRootPath().getUsableSpace()); + super(() -> getRootPath().getUsableSpace()); } private static File getRootPath() { diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index c753458ec557..de226f08910e 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -19,7 +19,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; - import org.enso.base.Stream_Utils; /** @@ -29,14 +28,12 @@ * deleting entries to make space for new ones. All cache files are set to be deleted automatically * on JVM exit. * - * Limits should be set with environment variables: - * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS -- single file size, in megs - * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT -- total cache size, in megs or percentage of free disk space + *

Limits should be set with environment variables: ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS -- + * single file size, in megs ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT -- total cache size, in megs + * or percentage of free disk space * - * Examples: - * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS=20 - * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=200 - * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=50% + *

Examples: ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS=20 + * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=200 ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=50% * * @param Additional metadata to associate with the data. */ @@ -44,13 +41,14 @@ public class LRUCache { private static final Logger logger = Logger.getLogger(LRUCache.class.getName()); /** - * An upper limit on the total cache size. If the cache size limit specified - * by the other parameters goes over this value, then this value is used. + * An upper limit on the total cache size. If the cache size limit specified by the other + * parameters goes over this value, then this value is used. */ private static final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND = 100L * 1024 * 1024 * 1024; /** Used to override cache parameters for testing. */ private final Map> cache = new HashMap<>(); + private final Map lastUsed = new HashMap<>(); /** Defines the per-file and total cache size limits. */ @@ -152,13 +150,13 @@ private File downloadResponseData(String cacheKey, Item item) try { // Limit the download to getMaxFileSize(). long maxFileSize = settings.getMaxFileSize(); - boolean sizeOK = Stream_Utils.limitedCopy(inputStream, outputStream, maxFileSize ); + boolean sizeOK = Stream_Utils.limitedCopy(inputStream, outputStream, maxFileSize); if (sizeOK) { successful = true; return temp; } else { - throw new ResponseTooLargeException(maxFileSize ); + throw new ResponseTooLargeException(maxFileSize); } } finally { outputStream.close(); @@ -259,17 +257,18 @@ private long getTotalCacheSize() { } /** - * Calculate the max total cache size, using the current limit but also - * constraining the result to the upper bound. + * Calculate the max total cache size, using the current limit but also constraining the result to + * the upper bound. */ public long getMaxTotalCacheSize() { - var totalCacheSize = switch (settings.getTotalCacheLimit()) { - case TotalCacheLimit.Bytes bytes -> bytes.bytes(); - case TotalCacheLimit.Percentage percentage -> { - long usableSpace = diskSpaceGetter.get(); - yield (long) (percentage.percentage() * usableSpace); - } - }; + var totalCacheSize = + switch (settings.getTotalCacheLimit()) { + case TotalCacheLimit.Bytes bytes -> bytes.bytes(); + case TotalCacheLimit.Percentage percentage -> { + long usableSpace = diskSpaceGetter.get(); + yield (long) (percentage.percentage() * usableSpace); + } + }; return Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java index 31308106f7d5..24f61b549e44 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -4,8 +4,8 @@ public class LRUCacheSettings { /** - * Default value for the largest file size allowed. - * Should be overridden with the ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS environment variable. + * Default value for the largest file size allowed. Should be overridden with the + * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS environment variable. */ private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; @@ -16,17 +16,16 @@ public class LRUCacheSettings { private static final double DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE = 0.2; /** - * Maximum size allowed for a single file. If a file larger than this is - * requested through this cache, a ResponseTooLargeException is thrown. + * Maximum size allowed for a single file. If a file larger than this is requested through this + * cache, a ResponseTooLargeException is thrown. */ private final long maxFileSize; /** * Limits the total size of all files in the cache. * - * This value can depend on free disk space, so it is not resolved to a - * maximum byte count at initialization time, but recalculated during each - * file cleanup. + *

This value can depend on free disk space, so it is not resolved to a maximum byte count at + * initialization time, but recalculated during each file cleanup. */ private final TotalCacheLimit.Limit totalCacheLimit; @@ -37,10 +36,16 @@ public LRUCacheSettings(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) /** Uses defaults if the vars are not set. */ public static LRUCacheSettings getDefault() { - String maxFileSizeSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); - String totalCacheLimitSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); - var maxFileSize = maxFileSizeSpec != null ? parseMaxFileSize(maxFileSizeSpec) : DEFAULT_MAX_FILE_SIZE; - var totalCacheLimit = totalCacheLimitSpec != null ? parseTotalCacheLimit(totalCacheLimitSpec) : new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + String maxFileSizeSpec = + Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); + String totalCacheLimitSpec = + Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); + var maxFileSize = + maxFileSizeSpec != null ? parseMaxFileSize(maxFileSizeSpec) : DEFAULT_MAX_FILE_SIZE; + var totalCacheLimit = + totalCacheLimitSpec != null + ? parseTotalCacheLimit(totalCacheLimitSpec) + : new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); return new LRUCacheSettings(maxFileSize, totalCacheLimit); } @@ -60,6 +65,6 @@ private static long parseMaxFileSize(String maxFileSizeSpec) { /** Uses the environment variable if set, otherwise uses a default percentage. */ private static TotalCacheLimit.Limit parseTotalCacheLimit(String totalCacheLimitSpec) { - return TotalCacheLimit.parse(totalCacheLimitSpec ); + return TotalCacheLimit.parse(totalCacheLimitSpec); } -} \ No newline at end of file +} diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index 876bc5df5c64..64e8ff838bf1 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -4,12 +4,12 @@ public class NowGetter extends Mockable { public NowGetter() { - super(() -> ZonedDateTime.now()); + super(() -> ZonedDateTime.now()); } /** - * This is necessary because a direct call to the superclass does not convert - * Value to ZonedDateTime. + * This is necessary because a direct call to the superclass does not convert Value to + * ZonedDateTime. */ @Override public void mocked(ZonedDateTime dt) { diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java index 8e993e13db09..ab319b20a3b4 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -4,36 +4,38 @@ import java.text.ParsePosition; /** - * Represents a limit on the total size of an LRUCache, either as a fixed byte - * count or as a percentage of available disk space. + * Represents a limit on the total size of an LRUCache, either as a fixed byte count or as a + * percentage of available disk space. */ public class TotalCacheLimit { - /** Parse the limit specification string into either a Bytes or Percentage value. */ - public static Limit parse(String limitString) throws NumberFormatException { - Number percentageNumber = tryPercentage(limitString); - if (percentageNumber != null) { - double percentage = percentageNumber.doubleValue(); - if (percentage < 0.0 || percentage > 1.0) { - throw new IllegalArgumentException("LURCache free disk space percentage must be in the range 0..100% (inclusive): was " + limitString); - } - return new Percentage(percentage); - } - double megs = Double.parseDouble(limitString); - long bytes = (long) (megs * 1024 * 1024); - return new Bytes(bytes); + /** Parse the limit specification string into either a Bytes or Percentage value. */ + public static Limit parse(String limitString) throws NumberFormatException { + Number percentageNumber = tryPercentage(limitString); + if (percentageNumber != null) { + double percentage = percentageNumber.doubleValue(); + if (percentage < 0.0 || percentage > 1.0) { + throw new IllegalArgumentException( + "LURCache free disk space percentage must be in the range 0..100% (inclusive): was " + + limitString); + } + return new Percentage(percentage); } + double megs = Double.parseDouble(limitString); + long bytes = (long) (megs * 1024 * 1024); + return new Bytes(bytes); + } - public sealed interface Limit permits Bytes, Percentage {} + public sealed interface Limit permits Bytes, Percentage {} - // Specify the limit in bytes. - public record Bytes(long bytes) implements Limit {} + // Specify the limit in bytes. + public record Bytes(long bytes) implements Limit {} - // Specify the limit as a percentage of total free, usable disk space. - public record Percentage(double percentage) implements Limit {} + // Specify the limit as a percentage of total free, usable disk space. + public record Percentage(double percentage) implements Limit {} - private static Number tryPercentage(String limitString) { - DecimalFormat df = new DecimalFormat("0%"); - ParsePosition pp = new ParsePosition(0); - return df.parse(limitString, pp); - } + private static Number tryPercentage(String limitString) { + DecimalFormat df = new DecimalFormat("0%"); + ParsePosition pp = new ParsePosition(0); + return df.parse(limitString, pp); + } } diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java index af40d57f5549..f9b5db483818 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoHTTPResponseCache.java @@ -3,10 +3,8 @@ import java.io.IOException; import java.io.InputStream; import java.net.http.HttpHeaders; -import java.util.List; import java.util.Optional; import org.enso.base.cache.LRUCache; -import org.enso.base.cache.TotalCacheLimit; import org.enso.base.cache.ResponseTooLargeException; /** @@ -23,8 +21,7 @@ */ public class EnsoHTTPResponseCache { // Public for testing. - public EnsoHTTPResponseCache() { - } + public EnsoHTTPResponseCache() {} // 1 year. private final int DEFAULT_TTL_SECONDS = 31536000; From 8a254695bb693e6703fdd67cc46006a534a31223 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 11 Nov 2024 15:52:35 -0500 Subject: [PATCH 30/40] include existing files in calc --- .../java/org/enso/base/cache/LRUCache.java | 18 ++++++++++++++---- test/Table_Tests/src/IO/Fetch_Spec.enso | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index de226f08910e..07d99eb7bead 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -221,8 +221,13 @@ private void removeCacheFile(String key, CacheEntry cacheEntry) { private void makeRoomFor(long newFileSize) { removeStaleEntries(); - long totalSize = getTotalCacheSize() + newFileSize; - long maxTotalCacheSize = getMaxTotalCacheSize(); + // Size of files on disk. + long currentCacheSize = getTotalCacheSize(); + // Upper limit to cache size. + long maxTotalCacheSize = getMaxTotalCacheSize(currentCacheSize); + // Size including new file. + long totalSize = currentCacheSize + newFileSize; + if (totalSize <= maxTotalCacheSize) { return; } @@ -260,18 +265,23 @@ private long getTotalCacheSize() { * Calculate the max total cache size, using the current limit but also constraining the result to * the upper bound. */ - public long getMaxTotalCacheSize() { + public long getMaxTotalCacheSize(long currentlyUsed) { var totalCacheSize = switch (settings.getTotalCacheLimit()) { case TotalCacheLimit.Bytes bytes -> bytes.bytes(); case TotalCacheLimit.Percentage percentage -> { - long usableSpace = diskSpaceGetter.get(); + long usableSpace = diskSpaceGetter.get() + currentlyUsed; yield (long) (percentage.percentage() * usableSpace); } }; return Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); } + /** For testing. */ + public long getMaxTotalCacheSize() { + return getMaxTotalCacheSize(getTotalCacheSize()); + } + public int getNumEntries() { return cache.size(); } diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index d01bfac93f1b..4475d1162c89 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -370,6 +370,22 @@ add_specs suite_builder = fetch_n 35 get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] + group_builder.specify "Includes the existing cache files in the total cache size calculation, for a percentage total cache limit" pending=pending_has_url <| Test.with_retries <| + with_config 1000 (TotalCacheLimit.Percentage.new 0.5) _-> disk_space-> + disk_space.mocked 100 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + fetch_n 30 + disk_space.mocked 70 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + fetch_n 20 + disk_space.mocked 50 + get_num_response_cache_entries . should_equal 2 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + fetch_n 10 + disk_space.mocked 70 + get_num_response_cache_entries . should_equal 2 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| with_config 1000 (TotalCacheLimit.Bytes.new 100) _-> _-> fetch_n 30 From 7137cbf34f8902fd552b1b878b168b4544667bee Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 11 Nov 2024 15:54:31 -0500 Subject: [PATCH 31/40] fmt --- std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 07d99eb7bead..802313335e78 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -226,7 +226,7 @@ private void makeRoomFor(long newFileSize) { // Upper limit to cache size. long maxTotalCacheSize = getMaxTotalCacheSize(currentCacheSize); // Size including new file. - long totalSize = currentCacheSize + newFileSize; + long totalSize = currentCacheSize + newFileSize; if (totalSize <= maxTotalCacheSize) { return; From f7940590af99c1a1bfc982cbe13be26abdd891d1 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 11 Nov 2024 15:58:51 -0500 Subject: [PATCH 32/40] doc --- .../base/src/main/java/org/enso/base/cache/NowGetter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index 64e8ff838bf1..88167201bd2e 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -8,8 +8,8 @@ public NowGetter() { } /** - * This is necessary because a direct call to the superclass does not convert Value to - * ZonedDateTime. + * This is necessary because a direct call to the superclass does not convert + * a polyglot Value to ZonedDateTime. */ @Override public void mocked(ZonedDateTime dt) { From 476a04cb4ea5ec5d33efd81915a4c6e2d74fe1b8 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Mon, 11 Nov 2024 16:54:58 -0500 Subject: [PATCH 33/40] fmt --- .../base/src/main/java/org/enso/base/cache/NowGetter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index 88167201bd2e..05637590a03f 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -8,8 +8,8 @@ public NowGetter() { } /** - * This is necessary because a direct call to the superclass does not convert - * a polyglot Value to ZonedDateTime. + * This is necessary because a direct call to the superclass does not convert a polyglot Value to + * ZonedDateTime. */ @Override public void mocked(ZonedDateTime dt) { From e559a0608c92787ff09a4c694dcca72b363a2539 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 13:27:24 -0500 Subject: [PATCH 34/40] 90% test, move a test --- test/Table_Tests/src/IO/Fetch_Spec.enso | 36 ++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 4475d1162c89..aba539c6f81f 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -370,22 +370,6 @@ add_specs suite_builder = fetch_n 35 get_cache_file_sizes . should_equal_ignoring_order [20, 35, 40] - group_builder.specify "Includes the existing cache files in the total cache size calculation, for a percentage total cache limit" pending=pending_has_url <| Test.with_retries <| - with_config 1000 (TotalCacheLimit.Percentage.new 0.5) _-> disk_space-> - disk_space.mocked 100 - EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 - fetch_n 30 - disk_space.mocked 70 - EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 - fetch_n 20 - disk_space.mocked 50 - get_num_response_cache_entries . should_equal 2 - EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 - fetch_n 10 - disk_space.mocked 70 - get_num_response_cache_entries . should_equal 2 - EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 - group_builder.specify "Will remove old cache files based on how recently they were used" pending=pending_has_url <| Test.with_retries <| with_config 1000 (TotalCacheLimit.Bytes.new 100) _-> _-> fetch_n 30 @@ -481,6 +465,22 @@ add_specs suite_builder = disk_space.mocked 400 EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 40 + group_builder.specify "Includes the existing cache files in the total cache size calculation, for a percentage total cache limit" pending=pending_has_url <| Test.with_retries <| + with_config 1000 (TotalCacheLimit.Percentage.new 0.5) _-> disk_space-> + disk_space.mocked 100 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + fetch_n 30 + disk_space.mocked 70 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + fetch_n 20 + disk_space.mocked 50 + get_num_response_cache_entries . should_equal 2 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + fetch_n 10 + disk_space.mocked 70 + get_num_response_cache_entries . should_equal 2 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 + group_builder.specify "Total cache size should not go over the hard-coded maximum" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| lru_cache = LRUCache.new @@ -490,8 +490,8 @@ add_specs suite_builder = Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| Test.expect_panic NumberFormatException LRUCache.new - group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| + group_builder.specify "Throws exception if the max total cache percentage is outside 0..90%" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| Test.expect_panic IllegalArgumentException LRUCache.new - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "103%" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "91%" <| Test.expect_panic IllegalArgumentException LRUCache.new From 08be1114ff014a130cbf8bfa10842bf9b4a53a89 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 14:18:10 -0500 Subject: [PATCH 35/40] review --- .../java/org/enso/base/cache/LRUCache.java | 26 ++++++++--------- test/Table_Tests/src/IO/Fetch_Spec.enso | 29 ++++++++++++++----- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index 802313335e78..a578ba807e79 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -35,6 +35,9 @@ *

Examples: ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS=20 * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=200 ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=50% * + * Regardless of other settings, the total cache size is capped at a percentage + * of the free disk space (MAX_PERCENTAGE). + * * @param Additional metadata to associate with the data. */ public class LRUCache { @@ -44,7 +47,7 @@ public class LRUCache { * An upper limit on the total cache size. If the cache size limit specified by the other * parameters goes over this value, then this value is used. */ - private static final long MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND = 100L * 1024 * 1024 * 1024; + private static final double MAX_PERCENTAGE = 0.9; /** Used to override cache parameters for testing. */ private final Map> cache = new HashMap<>(); @@ -94,6 +97,8 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB return new CacheResult<>(item.stream(), item.metadata()); } + // If we have a content-length, clear up enough space for that not. If not, + // then clear up enough space for the largest allowed file size. long maxFileSize = settings.getMaxFileSize(); if (item.sizeMaybe.isPresent()) { long size = item.sizeMaybe().get(); @@ -101,6 +106,8 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB throw new ResponseTooLargeException(maxFileSize); } makeRoomFor(size); + } else { + makeRoomFor(maxFileSize); } try { @@ -115,10 +122,6 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB cache.put(cacheKey, cacheEntry); markCacheEntryUsed(cacheKey); - // Clear out old entries to satisfy the total cache size limit. This might - // be necessary here if we didn't receive a correct content size value. - removeFilesToSatisfyLimit(); - return getResultForCacheEntry(cacheKey); } catch (IOException e) { logger.log( @@ -252,11 +255,6 @@ private SortedSet>> getSortedEntries() { return sortedEntries; } - /** Remove least-recently used entries until the total cache size is under the limit. */ - private void removeFilesToSatisfyLimit() { - makeRoomFor(0L); - } - private long getTotalCacheSize() { return cache.values().stream().collect(Collectors.summingLong(e -> e.size())); } @@ -266,15 +264,17 @@ private long getTotalCacheSize() { * the upper bound. */ public long getMaxTotalCacheSize(long currentlyUsed) { + long freeSpace = diskSpaceGetter.get() + currentlyUsed; + var totalCacheSize = switch (settings.getTotalCacheLimit()) { case TotalCacheLimit.Bytes bytes -> bytes.bytes(); case TotalCacheLimit.Percentage percentage -> { - long usableSpace = diskSpaceGetter.get() + currentlyUsed; - yield (long) (percentage.percentage() * usableSpace); + yield (long) (percentage.percentage() * freeSpace); } }; - return Long.min(MAX_TOTAL_CACHE_SIZE_FREE_SPACE_UPPER_BOUND, totalCacheSize); + long upperBound = (long) (freeSpace * MAX_PERCENTAGE); + return Long.min(upperBound, totalCacheSize); } /** For testing. */ diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index aba539c6f81f..1222adab5972 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -393,6 +393,16 @@ add_specs suite_builder = url = base_url_with_slash+'test_download?omit-content-length=1&length=110' Data.fetch url . should_fail_with (Response_Too_Large.Error 100) + group_builder.specify "Will make room for the largest possible file, if the server does not provide a content-length" pending=pending_has_url <| Test.with_retries <| + with_config 50 (TotalCacheLimit.Bytes.new 100) _-> _-> + fetch_n 20 + fetch_n 40 + fetch_n 10 + fetch_n 15 + url = base_url_with_slash+'test_download?omit-content-length=1&length=2' + Data.fetch url . should_succeed + get_cache_file_sizes . should_equal_ignoring_order [10, 15, 2] + group_builder.specify "Should not cache if the request fails" pending=pending_has_url <| Test.with_retries <| with_default_cache <| HTTP.fetch url0 @@ -448,7 +458,7 @@ add_specs suite_builder = lru_cache.getSettings.getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) lru_cache.getSettings.getTotalCacheLimit.percentage . should_equal 0.2 - group_builder.specify "Should be able to set the max file size and total cache size (in bytes) via environment variable" <| + group_builder.specify "Should be able to set the max file size and total cache size (in MB) via environment variable" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| with_default_cache <| @@ -481,17 +491,22 @@ add_specs suite_builder = get_num_response_cache_entries . should_equal 2 EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 50 - group_builder.specify "Total cache size should not go over the hard-coded maximum" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" (1024 * 1024).to_text <| - lru_cache = LRUCache.new - lru_cache.getMaxTotalCacheSize . should_equal (100 * 1024 * 1024 * 1024) + group_builder.specify "Total cache size, specified in MB, should not go over the percentage hard limit" <| + with_config 1000 (TotalCacheLimit.Bytes.new 200) _-> disk_space-> + disk_space.mocked 100 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 90 + + group_builder.specify "Total cache size, specified as a percentage, should not go over the percentage hard limit" <| + with_config 1000 (TotalCacheLimit.Percentage.new 0.95) _-> disk_space-> + disk_space.mocked 100 + EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 90 group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| Test.expect_panic NumberFormatException LRUCache.new - group_builder.specify "Throws exception if the max total cache percentage is outside 0..90%" <| + group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| Test.expect_panic IllegalArgumentException LRUCache.new - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "91%" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "101%" <| Test.expect_panic IllegalArgumentException LRUCache.new From 115e7c7dfe2e462bcd95bd12984abb5702fca12f Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 14:56:46 -0500 Subject: [PATCH 36/40] review --- .../org/enso/base/cache/DiskSpaceGetter.java | 4 ++-- .../java/org/enso/base/cache/LRUCache.java | 24 +++++++++++++------ .../org/enso/base/cache/LRUCacheSettings.java | 11 +++------ .../java/org/enso/base/cache/Mockable.java | 10 +++----- .../java/org/enso/base/cache/NowGetter.java | 4 ++-- test/Table_Tests/src/IO/Fetch_Spec.enso | 4 ++-- 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java index 758d7aa96d59..752316eb04a4 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java @@ -4,8 +4,8 @@ import org.enso.base.CurrentEnsoProject; public class DiskSpaceGetter extends Mockable { - public DiskSpaceGetter() { - super(() -> getRootPath().getUsableSpace()); + public Long computeValue() { + return getRootPath().getUsableSpace(); } private static File getRootPath() { diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index a578ba807e79..ebb5d95ef4d3 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -28,15 +28,25 @@ * deleting entries to make space for new ones. All cache files are set to be deleted automatically * on JVM exit. * - *

Limits should be set with environment variables: ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS -- - * single file size, in megs ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT -- total cache size, in megs - * or percentage of free disk space + *

Limits should be set with environment variables: * - *

Examples: ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS=20 - * ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=200 ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=50% + *

* - * Regardless of other settings, the total cache size is capped at a percentage - * of the free disk space (MAX_PERCENTAGE). + * 

+ * - ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB: single file size, in MB
+ * - ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT: total cache size, in MB or
+ *   percentage of free disk space
+ *
+ * + *

Examples: + * + * 

+ *   ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB=20
+ *   ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=200 ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT=50%
+ *
+ * + *

Regardless of other settings, the total cache size is capped at a percentage of the free disk + * space (MAX_PERCENTAGE). * * @param Additional metadata to associate with the data. */ diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java index 24f61b549e44..7fe855527a57 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -5,7 +5,7 @@ public class LRUCacheSettings { /** * Default value for the largest file size allowed. Should be overridden with the - * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS environment variable. + * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB environment variable. */ private static final long DEFAULT_MAX_FILE_SIZE = 2L * 1024 * 1024 * 1024; @@ -37,14 +37,14 @@ public LRUCacheSettings(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) /** Uses defaults if the vars are not set. */ public static LRUCacheSettings getDefault() { String maxFileSizeSpec = - Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS"); + Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB"); String totalCacheLimitSpec = Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); var maxFileSize = maxFileSizeSpec != null ? parseMaxFileSize(maxFileSizeSpec) : DEFAULT_MAX_FILE_SIZE; var totalCacheLimit = totalCacheLimitSpec != null - ? parseTotalCacheLimit(totalCacheLimitSpec) + ? TotalCacheLimit.parse(totalCacheLimitSpec) : new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); return new LRUCacheSettings(maxFileSize, totalCacheLimit); } @@ -62,9 +62,4 @@ private static long parseMaxFileSize(String maxFileSizeSpec) { double maxFileSizeMegs = Double.parseDouble(maxFileSizeSpec); return (long) (maxFileSizeMegs * 1024 * 1024); } - - /** Uses the environment variable if set, otherwise uses a default percentage. */ - private static TotalCacheLimit.Limit parseTotalCacheLimit(String totalCacheLimitSpec) { - return TotalCacheLimit.parse(totalCacheLimitSpec); - } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java b/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java index e2a0456de46d..8acf524db477 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/Mockable.java @@ -1,15 +1,11 @@ package org.enso.base.cache; import java.util.Optional; -import java.util.function.Supplier; -public class Mockable { - private Supplier supplier; +public abstract class Mockable { private Optional override = Optional.empty(); - public Mockable(Supplier supplier) { - this.supplier = supplier; - } + public abstract T computeValue(); public void mocked(T t) { this.override = Optional.of(t); @@ -20,6 +16,6 @@ public void unmocked() { } public T get() { - return override.orElse(supplier.get()); + return override.orElse(computeValue()); } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index 05637590a03f..5a39de6ffffd 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -3,8 +3,8 @@ import java.time.ZonedDateTime; public class NowGetter extends Mockable { - public NowGetter() { - super(() -> ZonedDateTime.now()); + public ZonedDateTime computeValue() { + return ZonedDateTime.now(); } /** diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 1222adab5972..3a01a3cbbec9 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -459,14 +459,14 @@ add_specs suite_builder = lru_cache.getSettings.getTotalCacheLimit.percentage . should_equal 0.2 group_builder.specify "Should be able to set the max file size and total cache size (in MB) via environment variable" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB" "8" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "30" <| with_default_cache <| EnsoSecretHelper.getCache.getLRUCache.getSettings.getMaxFileSize . should_equal (8 * 1024 * 1024) EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal (30 * 1024 * 1024) group_builder.specify "Should be able to set the max file size and total cache size (as a percentage) via environment variable, and track changes to available disk space" <| - Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MEGS" "8" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB" "8" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "10%" <| with_mocks _-> disk_space-> EnsoSecretHelper.getCache.getLRUCache.getSettings.getMaxFileSize . should_equal (8 * 1024 * 1024) From 6d679dcaeaa7230946a4f56251e5b2ab20eecb75 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 15:12:53 -0500 Subject: [PATCH 37/40] review --- .../base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java | 1 + std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java | 2 +- std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java index 752316eb04a4..4b797c678bd5 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/DiskSpaceGetter.java @@ -4,6 +4,7 @@ import org.enso.base.CurrentEnsoProject; public class DiskSpaceGetter extends Mockable { + @Override public Long computeValue() { return getRootPath().getUsableSpace(); } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java index ebb5d95ef4d3..4e17b51565f0 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java @@ -107,7 +107,7 @@ private CacheResult makeRequestAndCache(String cacheKey, ItemBuilder itemB return new CacheResult<>(item.stream(), item.metadata()); } - // If we have a content-length, clear up enough space for that not. If not, + // If we have a content-length, clear up enough space for that. If not, // then clear up enough space for the largest allowed file size. long maxFileSize = settings.getMaxFileSize(); if (item.sizeMaybe.isPresent()) { diff --git a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java index 5a39de6ffffd..d12bfc65e189 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/NowGetter.java @@ -3,6 +3,7 @@ import java.time.ZonedDateTime; public class NowGetter extends Mockable { + @Override public ZonedDateTime computeValue() { return ZonedDateTime.now(); } From 4e347e99bf14167ccb6c9e83142219fc654d1cb0 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 15:18:29 -0500 Subject: [PATCH 38/40] create cache on first use --- .../org/enso/base/enso_cloud/EnsoSecretHelper.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 6516b2797a7b..9859ab54cc7e 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -22,7 +22,7 @@ /** Makes HTTP requests with secrets in either header or query string. */ public final class EnsoSecretHelper extends SecretValueResolver { - private static final EnsoHTTPResponseCache cache = new EnsoHTTPResponseCache(); + private static EnsoHTTPResponseCache cache; /** Gets a JDBC connection resolving EnsoKeyValuePair into the properties. */ public static Connection getJDBCConnection( @@ -88,7 +88,7 @@ public static EnsoHttpResponse makeRequest( if (!useCache) { return requestMaker.makeRequest(); } else { - return cache.makeRequest(requestMaker); + return getOrCreateCache().makeRequest(requestMaker); } } @@ -176,6 +176,13 @@ public EnsoHttpResponse reconstructResponseFromCachedStream( } } + private static EnsoHTTPResponseCache getOrCreateCache() { + if (cache == null) { + cache = new EnsoHTTPResponseCache(); + } + return cache; + } + public static EnsoHTTPResponseCache getCache() { return cache; } From c5ab5126f68d5b9f0ff6af8f107328cbc3e29505 Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 15:50:23 -0500 Subject: [PATCH 39/40] default instead of exception for bad env vars --- .../org/enso/base/cache/LRUCacheSettings.java | 56 ++++++++++++++----- .../org/enso/base/cache/TotalCacheLimit.java | 2 +- test/Table_Tests/src/IO/Fetch_Spec.enso | 12 ++-- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java index 7fe855527a57..b1f4e76ab81f 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -1,8 +1,15 @@ package org.enso.base.cache; +import java.util.logging.Level; +import java.util.logging.Logger; import org.enso.base.Environment_Utils; public class LRUCacheSettings { + private static final Logger logger = Logger.getLogger(LRUCacheSettings.class.getName()); + + private static final String MAX_FILE_SIZE_ENV_VAR = "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB"; + private static final String TOTAL_CACHE_SIZE_ENV_VAR = "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"; + /** * Default value for the largest file size allowed. Should be overridden with the * ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB environment variable. @@ -36,17 +43,7 @@ public LRUCacheSettings(long maxFileSize, TotalCacheLimit.Limit totalCacheLimit) /** Uses defaults if the vars are not set. */ public static LRUCacheSettings getDefault() { - String maxFileSizeSpec = - Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB"); - String totalCacheLimitSpec = - Environment_Utils.get_environment_variable("ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"); - var maxFileSize = - maxFileSizeSpec != null ? parseMaxFileSize(maxFileSizeSpec) : DEFAULT_MAX_FILE_SIZE; - var totalCacheLimit = - totalCacheLimitSpec != null - ? TotalCacheLimit.parse(totalCacheLimitSpec) - : new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); - return new LRUCacheSettings(maxFileSize, totalCacheLimit); + return new LRUCacheSettings(parseMaxFileSizeEnvVar(), parseTotalCacheLimitEnvVar()); } public long getMaxFileSize() { @@ -57,9 +54,38 @@ public TotalCacheLimit.Limit getTotalCacheLimit() { return totalCacheLimit; } - /** Uses the environment variable if set, otherwise uses a default. */ - private static long parseMaxFileSize(String maxFileSizeSpec) { - double maxFileSizeMegs = Double.parseDouble(maxFileSizeSpec); - return (long) (maxFileSizeMegs * 1024 * 1024); + // Uses the environment variable if set and correctly formatted, otherwise + // uses a default. + private static long parseMaxFileSizeEnvVar() { + String maxFileSizeSpec = + Environment_Utils.get_environment_variable(MAX_FILE_SIZE_ENV_VAR); + if (maxFileSizeSpec == null) { + return DEFAULT_MAX_FILE_SIZE; + } + try { + double maxFileSizeMegs = Double.parseDouble(maxFileSizeSpec); + return (long) (maxFileSizeMegs * 1024 * 1024); + } catch (NumberFormatException e) { + logger.log( + Level.WARNING, "Unable to parse environment variable " + MAX_FILE_SIZE_ENV_VAR + ": {}, falling back to default", e); + return DEFAULT_MAX_FILE_SIZE; + } + } + + // Uses the environment variable if set and correctly formatted, otherwise + // uses a default. + private static TotalCacheLimit.Limit parseTotalCacheLimitEnvVar() { + String totalCacheLimitSpec = + Environment_Utils.get_environment_variable(TOTAL_CACHE_SIZE_ENV_VAR); + if (totalCacheLimitSpec == null) { + return new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + } + try { + return TotalCacheLimit.parse(totalCacheLimitSpec); + } catch (IllegalArgumentException e) { + logger.log( + Level.WARNING, "Unable to parse environment variable " + TOTAL_CACHE_SIZE_ENV_VAR + ": {}, falling back to default", e); + return new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); + } } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java index ab319b20a3b4..e211b0202097 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -9,7 +9,7 @@ */ public class TotalCacheLimit { /** Parse the limit specification string into either a Bytes or Percentage value. */ - public static Limit parse(String limitString) throws NumberFormatException { + public static Limit parse(String limitString) throws IllegalArgumentException, NumberFormatException { Number percentageNumber = tryPercentage(limitString); if (percentageNumber != null) { double percentage = percentageNumber.doubleValue(); diff --git a/test/Table_Tests/src/IO/Fetch_Spec.enso b/test/Table_Tests/src/IO/Fetch_Spec.enso index 3a01a3cbbec9..bb809ffb379a 100644 --- a/test/Table_Tests/src/IO/Fetch_Spec.enso +++ b/test/Table_Tests/src/IO/Fetch_Spec.enso @@ -501,12 +501,14 @@ add_specs suite_builder = disk_space.mocked 100 EnsoSecretHelper.getCache.getLRUCache.getMaxTotalCacheSize . should_equal 90 - group_builder.specify "Throws exception if the max total cache size is incorrectly formatted" <| + group_builder.specify "Falls back to the default if an environment variable is incorrectly formatted" <| + Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB" "abcd" <| + LRUCache.new . getSettings . getMaxFileSize . should_equal (2 * 1024 * 1024 * 1024) Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "50q.%" <| - Test.expect_panic NumberFormatException LRUCache.new + LRUCache.new . getSettings . getTotalCacheLimit . should_equal (TotalCacheLimit.Percentage.new 0.2) - group_builder.specify "Throws exception if the max total cache percentage is outside 0..100%" <| + group_builder.specify "Falls back to the default if the max total cache percentage is outside 0..100%" <| Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "-10%" <| - Test.expect_panic IllegalArgumentException LRUCache.new + LRUCache.new . getSettings . getTotalCacheLimit . should_equal (TotalCacheLimit.Percentage.new 0.2) Test_Environment.unsafe_with_environment_override "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT" "101%" <| - Test.expect_panic IllegalArgumentException LRUCache.new + LRUCache.new . getSettings . getTotalCacheLimit . should_equal (TotalCacheLimit.Percentage.new 0.2) From d2d7b295bd941634ff0f1b0a7ecf2f4ea79b838c Mon Sep 17 00:00:00 2001 From: Gregory Travis Date: Tue, 12 Nov 2024 15:52:43 -0500 Subject: [PATCH 40/40] fmt --- .../org/enso/base/cache/LRUCacheSettings.java | 18 +++++++++++++----- .../org/enso/base/cache/TotalCacheLimit.java | 3 ++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java index b1f4e76ab81f..2678b9add3ee 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/LRUCacheSettings.java @@ -8,7 +8,8 @@ public class LRUCacheSettings { private static final Logger logger = Logger.getLogger(LRUCacheSettings.class.getName()); private static final String MAX_FILE_SIZE_ENV_VAR = "ENSO_LIB_HTTP_CACHE_MAX_FILE_SIZE_MB"; - private static final String TOTAL_CACHE_SIZE_ENV_VAR = "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"; + private static final String TOTAL_CACHE_SIZE_ENV_VAR = + "ENSO_LIB_HTTP_CACHE_MAX_TOTAL_CACHE_LIMIT"; /** * Default value for the largest file size allowed. Should be overridden with the @@ -57,8 +58,7 @@ public TotalCacheLimit.Limit getTotalCacheLimit() { // Uses the environment variable if set and correctly formatted, otherwise // uses a default. private static long parseMaxFileSizeEnvVar() { - String maxFileSizeSpec = - Environment_Utils.get_environment_variable(MAX_FILE_SIZE_ENV_VAR); + String maxFileSizeSpec = Environment_Utils.get_environment_variable(MAX_FILE_SIZE_ENV_VAR); if (maxFileSizeSpec == null) { return DEFAULT_MAX_FILE_SIZE; } @@ -67,7 +67,11 @@ private static long parseMaxFileSizeEnvVar() { return (long) (maxFileSizeMegs * 1024 * 1024); } catch (NumberFormatException e) { logger.log( - Level.WARNING, "Unable to parse environment variable " + MAX_FILE_SIZE_ENV_VAR + ": {}, falling back to default", e); + Level.WARNING, + "Unable to parse environment variable " + + MAX_FILE_SIZE_ENV_VAR + + ": {}, falling back to default", + e); return DEFAULT_MAX_FILE_SIZE; } } @@ -84,7 +88,11 @@ private static TotalCacheLimit.Limit parseTotalCacheLimitEnvVar() { return TotalCacheLimit.parse(totalCacheLimitSpec); } catch (IllegalArgumentException e) { logger.log( - Level.WARNING, "Unable to parse environment variable " + TOTAL_CACHE_SIZE_ENV_VAR + ": {}, falling back to default", e); + Level.WARNING, + "Unable to parse environment variable " + + TOTAL_CACHE_SIZE_ENV_VAR + + ": {}, falling back to default", + e); return new TotalCacheLimit.Percentage(DEFAULT_TOTAL_CACHE_SIZE_FREE_SPACE_PERCENTAGE); } } diff --git a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java index e211b0202097..1ef2a5b5e8ab 100644 --- a/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java +++ b/std-bits/base/src/main/java/org/enso/base/cache/TotalCacheLimit.java @@ -9,7 +9,8 @@ */ public class TotalCacheLimit { /** Parse the limit specification string into either a Bytes or Percentage value. */ - public static Limit parse(String limitString) throws IllegalArgumentException, NumberFormatException { + public static Limit parse(String limitString) + throws IllegalArgumentException, NumberFormatException { Number percentageNumber = tryPercentage(limitString); if (percentageNumber != null) { double percentage = percentageNumber.doubleValue();