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

Add an optional filter to limit artifact sizes by Action #1551

Open
shirchen opened this issue Nov 20, 2023 · 4 comments
Open

Add an optional filter to limit artifact sizes by Action #1551

shirchen opened this issue Nov 20, 2023 · 4 comments

Comments

@shirchen
Copy link
Contributor

Currently, buildfarm only offers a global configuration on maxEntrySizeBytes which will reject with code 11 any Write requests that are greater than value set in maxEntrySizeBytes.

We would like to propose to add an optional filter for Actions which we would allow to exceed a specified limit. Use case is that we have some incremental actions (eg Docker containers) that rarely change we expect to get a good cache hit rate from them. At the same time, we want to keep any new rules under the global limit.

Implementation can do achieved by adding a configuration

https://github.com/bazelbuild/bazel-buildfarm/blob/main/src/main/java/build/buildfarm/cas/cfc/CASFileCache.java#L793
And then filter by RequestMetadata which already has an ActionMnemonic which we can use.

@werkt
Copy link
Member

werkt commented Nov 21, 2023

So the value to be compared against the limit is "the sum of all file sizes for the tree of inputs to an action"?

@shirchen
Copy link
Contributor Author

I was thinking that we would limit just on the digest's size with values read from configuration.

--- a/src/main/java/build/buildfarm/cas/cfc/CASFileCache.java
+++ b/src/main/java/build/buildfarm/cas/cfc/CASFileCache.java
@@ -790,7 +790,11 @@ public abstract class CASFileCache implements ContentAddressableStorage {
     if (digest.getSizeBytes() == 0) {
       return new CompleteWrite(0);
     }
-    if (digest.getSizeBytes() > maxEntrySizeInBytes) {
+    Set<String> allowedActions = Sets.newConcurrentHashSet();
+    allowedActions.add("JoinLayers");
+    allowedActions.add("PackageTar");
+
+    if (digest.getSizeBytes() > maxEntrySizeInBytes && !allowedActions.contains(requestMetadata.getActionMnemonic())) {
       throw new EntryLimitException(digest.getSizeBytes(), maxEntrySizeInBytes);
     }
     try {

@werkt
Copy link
Member

werkt commented Nov 21, 2023

"the digest" as a subject for comparison does not narrow the scope of the filter - we already limit by size generally, and ensure that no blobs can exist in a worker's cas above that limit.

Your suggested change limits by hard coded action mnemonics. What does that have anything to do with 'size'?

@shirchen
Copy link
Contributor Author

Your suggested change limits by hard coded action mnemonics. What does that have anything to do with 'size'?

Good question. From my prototype snippet above, I'm proposing allowing some actions to go above the hardcoded size based on configured list of mnemonics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants