Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP cache size limit environment variables #11530

Merged
merged 47 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
d3eb7fa
wip
GregoryTravis Oct 31, 2024
3b3848c
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 1, 2024
8064cfb
make EHRC static
GregoryTravis Nov 1, 2024
65d6141
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 4, 2024
12c3733
EHRC singleton
GregoryTravis Nov 4, 2024
8f7c0ab
wip
GregoryTravis Nov 4, 2024
54cad98
wip
GregoryTravis Nov 4, 2024
eac9637
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 5, 2024
c071438
tests pass
GregoryTravis Nov 5, 2024
232c88b
use enso project root
GregoryTravis Nov 5, 2024
c744a58
docs, rename
GregoryTravis Nov 5, 2024
ad57bd7
docs, test for changing disk space
GregoryTravis Nov 5, 2024
5742258
doc
GregoryTravis Nov 5, 2024
859459a
doc
GregoryTravis Nov 5, 2024
875aa1f
prevent raising test disk space
GregoryTravis Nov 5, 2024
36b3867
upper bound test
GregoryTravis Nov 5, 2024
e5dc3a5
wip
GregoryTravis Nov 5, 2024
878d992
wip
GregoryTravis Nov 5, 2024
55379f4
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 6, 2024
6a896f0
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 8, 2024
0d9da62
wip
GregoryTravis Nov 8, 2024
d62ffa9
wip
GregoryTravis Nov 8, 2024
0321544
one passes
GregoryTravis Nov 8, 2024
a1be3e6
17
GregoryTravis Nov 8, 2024
31a8f02
double lambda
GregoryTravis Nov 8, 2024
4a87a25
fix now
GregoryTravis Nov 8, 2024
0e44c66
more
GregoryTravis Nov 8, 2024
edc18cb
green
GregoryTravis Nov 8, 2024
3b079f9
do not have to set both env vars
GregoryTravis Nov 8, 2024
3d2fcf9
download not limited check checks that fetch throws
GregoryTravis Nov 8, 2024
141d6a2
file_deleter, fetch_n
GregoryTravis Nov 8, 2024
46634f5
deleter fix
GregoryTravis Nov 8, 2024
8500533
wip
GregoryTravis Nov 8, 2024
6d420a2
fmt
GregoryTravis Nov 8, 2024
8a25469
include existing files in calc
GregoryTravis Nov 11, 2024
7137cbf
fmt
GregoryTravis Nov 11, 2024
e31c107
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 11, 2024
f794059
doc
GregoryTravis Nov 11, 2024
476a04c
fmt
GregoryTravis Nov 11, 2024
c1da246
Merge branch 'develop' into wip/gmt/11410-cache-env
GregoryTravis Nov 12, 2024
e559a06
90% test, move a test
GregoryTravis Nov 12, 2024
08be111
review
GregoryTravis Nov 12, 2024
115e7c7
review
GregoryTravis Nov 12, 2024
6d679dc
review
GregoryTravis Nov 12, 2024
4e347e9
create cache on first use
GregoryTravis Nov 12, 2024
c5ab512
default instead of exception for bad env vars
GregoryTravis Nov 12, 2024
d2d7b29
fmt
GregoryTravis Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +94 to +97
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit redundant, since we can just call invokeMethod twice on Java side, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because invokeMember didn't work here; I get:

Exception occurred in target VM: Unsupported operation Value.invoke(path, Object...) for '(File /Users/gmt/dev/enso/enso/test/Table_Tests)'(language: Java, type: org.graalvm.polyglot.Value). You can ensure that the operation is supported using Value.canInvoke(String).

Perhaps because it's a builtin Enso type?

root.hasMember("path")
false

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. Well in such case such workaround seems justified.


## 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.getCache.clear

## PRIVATE
ADVANCED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,32 @@
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;
Expand All @@ -46,6 +52,10 @@ public String getNamespace() {
return namespace;
}

public String getRootPath() {
return rootPath;
}

public String fullName() {
return namespace + "." + name;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.enso.base.cache;

import java.io.File;
import org.enso.base.CurrentEnsoProject;

public class DiskSpaceGetter extends Mockable<Long> {
@Override
public Long computeValue() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably add @Override at the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return getRootPath().getUsableSpace();
}

private static File getRootPath() {
return new File(CurrentEnsoProject.get().getRootPath());
}
}
185 changes: 88 additions & 97 deletions std-bits/base/src/main/java/org/enso/base/cache/LRUCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,59 @@
* deleting entries to make space for new ones. All cache files are set to be deleted automatically
* on JVM exit.
*
* <p>Limits should be set with environment variables:
*
* <p>
*
* <pre>
* - 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
* </pre>
*
* <p>Examples:
*
* <pre>
* 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%
* </pre>
*
* <p>Regardless of other settings, the total cache size is capped at a percentage of the free disk
* space (MAX_PERCENTAGE).
*
* @param <M> Additional metadata to associate with the data.
*/
public class LRUCache<M> {
private static final Logger logger = Logger.getLogger(LRUCache.class.getName());

private final long maxFileSize;
private final long maxTotalCacheSize;

private final CacheTestParameters cacheTestParameters = new CacheTestParameters();
/**
* 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 double MAX_PERCENTAGE = 0.9;

/** Used to override cache parameters for testing. */
private final Map<String, CacheEntry<M>> cache = new HashMap<>();

private final Map<String, ZonedDateTime> lastUsed = new HashMap<>();

public LRUCache(long maxFileSize, long maxTotalCacheSize) {
this.maxFileSize = maxFileSize;
this.maxTotalCacheSize = maxTotalCacheSize;
/** 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(LRUCacheSettings.getDefault(), new NowGetter(), new DiskSpaceGetter());
}

public LRUCache(LRUCacheSettings settings, NowGetter nowGetter, DiskSpaceGetter diskSpaceGetter) {
this.settings = settings;
this.nowGetter = nowGetter;
this.diskSpaceGetter = diskSpaceGetter;
}

public CacheResult<M> getResult(ItemBuilder<M> itemBuilder)
Expand All @@ -70,30 +107,31 @@ private CacheResult<M> makeRequestAndCache(String cacheKey, ItemBuilder<M> itemB
return new CacheResult<>(item.stream(), item.metadata());
}

// 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()) {
long size = item.sizeMaybe().get();
if (size > getMaxFileSize()) {
throw new ResponseTooLargeException(getMaxFileSize());
if (size > maxFileSize) {
throw new ResponseTooLargeException(maxFileSize);
}
makeRoomFor(size);
} else {
makeRoomFor(maxFileSize);
}

try {
// Download the response data.
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);
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(
Expand Down Expand Up @@ -124,13 +162,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 = settings.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();
Expand All @@ -144,12 +183,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));
}

Expand Down Expand Up @@ -195,8 +234,13 @@ private void removeCacheFile(String key, CacheEntry<M> 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;
}
Expand All @@ -221,39 +265,46 @@ private SortedSet<Map.Entry<String, CacheEntry<M>>> 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()));
}

private long getMaxFileSize() {
return cacheTestParameters.getMaxFileSizeOverrideTestOnly().orElse(maxFileSize);
/**
* Calculate the max total cache size, using the current limit but also constraining the result to
* 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 -> {
yield (long) (percentage.percentage() * freeSpace);
}
};
long upperBound = (long) (freeSpace * MAX_PERCENTAGE);
return Long.min(upperBound, totalCacheSize);
}

private long getMaxTotalCacheSize() {
return cacheTestParameters.getMaxTotalCacheSizeOverrideTestOnly().orElse(maxTotalCacheSize);
/** For testing. */
public long getMaxTotalCacheSize() {
return getMaxTotalCacheSize(getTotalCacheSize());
}

public int getNumEntries() {
return cache.size();
}

public List<Long> getFileSizesTestOnly() {
/** Public for testing. */
public List<Long> getFileSizes() {
return new ArrayList<>(
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;
/** Public for testing. */
public LRUCacheSettings getSettings() {
return settings;
}

private record CacheEntry<M>(File responseData, M metadata, long size, ZonedDateTime expiry) {}
Expand Down Expand Up @@ -290,64 +341,4 @@ public interface ItemBuilder<M> {

private final Comparator<Map.Entry<String, CacheEntry<M>>> cacheEntryLRUComparator =
Comparator.comparing(me -> lastUsed.get(me.getKey()));

/** A set of parameters that can be used to modify cache settings for testing purposes. */
public class CacheTestParameters {
/** This value is used for the current time when testing TTL expiration logic. */
private Optional<ZonedDateTime> nowOverrideTestOnly = Optional.empty();

/**
* Used for testing file and cache size limits. These cannot be set to values larger than the
* real limits.
*/
private Optional<Long> maxFileSizeOverrideTestOnly = Optional.empty();

private Optional<Long> maxTotalCacheSizeOverrideTestOnly = Optional.empty();

public Optional<ZonedDateTime> getNowOverrideTestOnly() {
return nowOverrideTestOnly;
}

public void setNowOverrideTestOnly(ZonedDateTime nowOverride) {
nowOverrideTestOnly = Optional.of(nowOverride);
}

public void clearNowOverrideTestOnly() {
nowOverrideTestOnly = Optional.empty();
}

public Optional<Long> 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<Long> getMaxTotalCacheSizeOverrideTestOnly() {
return maxTotalCacheSizeOverrideTestOnly;
}

public void setMaxTotalCacheSizeOverrideTestOnly(long maxTotalCacheSizeOverrideTestOnly_) {
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();
}
}
}
Loading
Loading