Repository: bazelbuild/bazel
Issue: improve --experimental_local_disk_cache
Labels: type: feature request
P3

Many users like the `--experimental_local_disk_cache` option, but it needs some improvements.

- Replace the two options `--experimental_local_disk_cache` and `--experimental_local_disk_cache_path` with one `--local_disk_cache=/path/to/disk/cache`.
- Add a `--local_disk_cache_size=SIZE` option to specify an upper bound on the folder. A LRU cache is probably fine. We'll probably need to introduce a `lockfile` to make sure only one Bazel server uses the cache at a time.
- Document it. Probably https://docs.bazel.build/versions/master/remote-caching.html is a good place.

cc: @davido 

Issue comments:
Actuall there are three options. The one you havn't mentioned: `--experimental_strict_action_env`. Without this options, the local action cache is useless, if the workspace location changed (clone project in two different locations). See also discussion on the #3042, where local action cache was introduced.

I agree with your suggestions. I can look into renaming the options and I agree that to lose "experimental" part is a good idea. I also started to look into adding upper bound size limit to local action cache. Surprisingly, Bazel doesn't have any means for now to do that (no LRU cache or similar).



Why lock it?
On Mon, 19 Mar 2018 at 13:04 David Ostrovsky <notifications@github.com>
wrote:

> Actuall there are three options. The one you havn't mentioned:
> --experimental_strict_action_env. Without this options, the caches is
> useless, if the workspace changed (clone project in two different
> locations). See also discussion on the original issue where local action
> cache was introduced.
>
> I agree with your suggestions. I can look into renaming the options and I
> agree that we lose experimental part is a good idea. I also started to look
> into adding upper bound size limit to local action cache. Surprisingly,
> Bazel doesn't have any means for now to do that (no LRU cache or similar).
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/4870#issuecomment-374175356>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABUIF4Sgargwlf86MAnobdRBr5OxKHqoks5tf5CkgaJpZM4Sv7KB>
> .
>

@davido good point, but --experimental_strict_action_env is not specific to the local disk cache. We should certainly document it.

Yes, Bazel currently doesn't have that, *but* the remote module could just have it's own LRU cache that it keeps in memory while the server is running. That should be sufficient I think.

@ittaiz Not a strict requirement, I just imagine it to be difficult to keep the LRU cache in sync with multiple Bazel's deleting from it concurrently.
@davido would you want to work on this? 😃 
@buchgr Yes, feel free to assign it to me.
Should we also have it respect the `no-cache` options as the remote-cache has? [exclude-specific-targets-from-using-the-remote-cache](https://docs.bazel.build/versions/master/remote-caching.html#exclude-specific-targets-from-using-the-remote-cache)?
Is there a timeline for this feature?
@promiseofcake yes we should.

@RNabel we love contributions :-)
@davido There are a few of your commits referenced here -- are you still working on this?
@RNabel There is a pending change for review: https://bazel-review.googlesource.com/c/bazel/+/53810.

However, all this change does, is to consolidate already working local disk cache options. [Gerrit Code Review](https://github.com/GerritCodeReview) projects is using this feature for many months now. See the documentation how to activate it (you can pick any paths):

https://github.com/GerritCodeReview/gerrit/blob/master/Documentation/dev-bazel.txt#L366-L389

Moreover, in very recent Bazel version, [I added the ability](https://github.com/bazelbuild/bazel/pull/4852) to specify `$HOME` user directory for local action cache. So that when this CL is included in bazel release, we would be able to commit this configuration in the repository and thus activate local action cache per default (instead of providing documentation in our build tool chain how to activate the local action cache, that nobody reading anyway):

```
build --experimental_local_disk_cache_path=~/.gerritcodereview/bazel-cache/cas
```


@davido's changes have just been merged ... yay! https://github.com/bazelbuild/bazel/commit/4ee7f114387187c8b578b7f41674b9e68593d20e 

I ll open another issue for discussing garbage collection.
#5139

Pull request review comments:
I wonder if we should just call this feature "Disk Cache". I think "local action cache" is confusing because it's not only caching actions but also the output and I think it's easy to confuse with Bazel's actual action cache.

So what do you think about calling this "Disk Cache" and the flag `--disk_cache`?
Also, what do you think about moving this after "External Links" and before "Bazel remote execution (in development)"?
I think we should also rename this test to `disk_cache_test.sh`?
"A path to a directory where Bazel can read and write actions and action outputs. If the directory does not exist, it will be created."?
I know it's a bit out of scope of this PR, but can you please also fix that Bazel will create the directory if it does not exist?
Disk Cache?
I rephrased your words a bit to fit in better with the rest of the document:

Bazel can use a directory on the filesystem as a remote cache. This is
useful for sharing build artifacts when switching branches and/or working
on multiple workspaces of the same project i.e. multiple checkouts. Bazel
does not garbage collect the directory and thus one might want to set up a
cron job or similar to periodically clean up the directory. The disk cache
can be enabled with the following flags.

```
build --experimental_remote_spawn_cache
build --disk_cache=/path/to/disk/cache
``` 

Additionally, one can pass a user specific path to the `--disk_cache` flag
via the `~` character. Bazel will replace this with the current user's home
directory. This comes in handy when one wants to enable the disk cache for
all developers of a project via the project's checked in `.bazelrc` file.

Why is this important for different workspaces? These environment variables should be the same for one machine and user? Are you thinking about the case where multiple users use the same cache?
Done.
Done.
I removed the listing of the env. variables. The use case I'm after is: the same user (no multiple uses involved) has different clones of the same project, say:

* /home/davido/projects/gerrit
* /home/davido/projects/gerrit2

After building in gerrit directory, the cache is populated. When I build in gerrit2 directory, without `--experimental_strict_action_env` I experienced cache misses. I havn't re-checked whether this is still the case, but @ulfjack recommended to use this option in this comment: https://github.com/bazelbuild/bazel/issues/3042#issuecomment-330151933 and pointed to the discussion in context of this issue: https://github.com/bazelbuild/bazel/issues/2574.

I should have probably said: "To have cache hits across different clones of the same project..."?

Patch that resolved the issue:
diff --git a/site/docs/remote-caching.md b/site/docs/remote-caching.md
index 163f1122f2ad69..a256240cbd7cb6 100644
--- a/site/docs/remote-caching.md
+++ b/site/docs/remote-caching.md
@@ -28,6 +28,7 @@ make builds significantly faster.
     * [Delete content from the remote cache](#delete-content-from-the-remote-cache)
 * [Known Issues](#known-issues)
 * [External Links](#external-links)
+* [Build cache](#build-cache)
 * [Bazel remote execution (in development)](#bazel-remote-execution-in-development)
 
 ## Remote caching overview
@@ -306,6 +307,31 @@ You may want to delete content from the cache to:
 * Create a clean cache after a cache was poisoned
 * Reduce the amount of storage used by deleting old outputs
 
+## Build cache
+
+Bazel can use a directory on the filesystem as a remote cache. This is
+useful for sharing build artifacts when switching branches and/or working
+on multiple workspaces of the same project i.e. multiple checkouts. Bazel
+does not garbage collect the directory and thus one might want to set up a
+cron job or similar to periodically clean up the directory. The build cache
+can be enabled with the following flags.
+
+```
+build --build_cache=/path/to/build/cache
+```
+
+Additionally, one can pass a user specific path to the --build_cache flag
+via the ~ character. Bazel will replace this with the current user's home
+directory. This comes in handy when one wants to enable the build cache for
+all developers of a project via the project's checked in `.bazelrc` file.
+
+To have cache hits across different workspaces, additional option should be
+used:
+
+```
+build --experimental_strict_action_env
+```
+
 ## Known issues
 
 **Input file modification during a Build**
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index f3a2baec314327..16dc3f0bbce7b4 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -60,7 +60,7 @@ public Iterable<? extends ActionContext> getActionContexts() {
     String buildRequestId = env.getBuildRequestId().toString();
     String commandId = env.getCommandId().toString();
 
-    if (remoteOptions.experimentalRemoteSpawnCache || remoteOptions.experimentalLocalDiskCache) {
+    if (remoteOptions.experimentalRemoteSpawnCache || remoteOptions.buildCache != null) {
       RemoteSpawnCache spawnCache =
           new RemoteSpawnCache(
               env.getExecRoot(),
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
index b60241d5c798f6..35828724e9db4c 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -190,27 +190,18 @@ public final class RemoteOptions extends OptionsBase {
   )
   public boolean experimentalRemoteSpawnCache;
 
-  // TODO(davido): Find a better place for this and the next option.
+  // TODO(davido): Find a better place for this option.
   @Option(
-    name = "experimental_local_disk_cache",
-    defaultValue = "false",
-    category = "remote",
-    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-    effectTags = {OptionEffectTag.UNKNOWN},
-    help = "Whether to use the experimental local disk cache."
-  )
-  public boolean experimentalLocalDiskCache;
-
-  @Option(
-    name = "experimental_local_disk_cache_path",
+    name = "build_cache",
     defaultValue = "null",
     category = "remote",
     documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
     effectTags = {OptionEffectTag.UNKNOWN},
     converter = OptionsUtils.PathFragmentConverter.class,
-    help = "A file path to a local disk cache."
+    help = "A path to a directory where Bazel can read and write actions and action outputs. "
+        + "If the directory does not exist, it will be created."
   )
-  public PathFragment experimentalLocalDiskCachePath;
+  public PathFragment buildCache;
 
   @Option(
     name = "experimental_guard_against_concurrent_changes",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
index 70a323ffeb8ba7..1f35dfdafb85dc 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
@@ -46,10 +46,10 @@ public static SimpleBlobStore createRest(RemoteOptions options, Credentials cred
     }
   }
 
-  public static SimpleBlobStore createLocalDisk(RemoteOptions options, Path workingDirectory)
+  public static SimpleBlobStore createBuildCache(RemoteOptions options, Path workingDirectory)
       throws IOException {
     return new OnDiskBlobStore(
-        workingDirectory.getRelative(checkNotNull(options.experimentalLocalDiskCachePath)));
+        workingDirectory.getRelative(checkNotNull(options.buildCache)));
   }
 
   public static SimpleBlobStore create(
@@ -58,8 +58,8 @@ public static SimpleBlobStore create(
     if (isRestUrlOptions(options)) {
       return createRest(options, creds);
     }
-    if (workingDirectory != null && isLocalDiskCache(options)) {
-      return createLocalDisk(options, workingDirectory);
+    if (workingDirectory != null && isBuildCache(options)) {
+      return createBuildCache(options, workingDirectory);
     }
     throw new IllegalArgumentException(
         "Unrecognized concurrent map RemoteOptions: must specify "
@@ -67,11 +67,11 @@ public static SimpleBlobStore create(
   }
 
   public static boolean isRemoteCacheOptions(RemoteOptions options) {
-    return isRestUrlOptions(options) || isLocalDiskCache(options);
+    return isRestUrlOptions(options) || isBuildCache(options);
   }
 
-  public static boolean isLocalDiskCache(RemoteOptions options) {
-    return options.experimentalLocalDiskCache;
+  public static boolean isBuildCache(RemoteOptions options) {
+    return options.buildCache != null;
   }
 
   private static boolean isRestUrlOptions(RemoteOptions options) {


Test patch:
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index d271c43627b192..8f593e565bc11d 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -318,9 +318,9 @@ sh_test(
 )
 
 sh_test(
-    name = "local_action_cache_test",
+    name = "build_cache_test",
     size = "small",
-    srcs = ["local_action_cache_test.sh"],
+    srcs = ["build_cache_test.sh"],
     data = [":test-deps"],
 )
 
diff --git a/src/test/shell/bazel/local_action_cache_test.sh b/src/test/shell/bazel/build_cache_test.sh
similarity index 95%
rename from src/test/shell/bazel/local_action_cache_test.sh
rename to src/test/shell/bazel/build_cache_test.sh
index 60c9c4ecc1abf4..0333398c7bc7a2 100755
--- a/src/test/shell/bazel/local_action_cache_test.sh
+++ b/src/test/shell/bazel/build_cache_test.sh
@@ -27,7 +27,7 @@ function test_local_action_cache() {
   local execution_file="${TEST_TMPDIR}/run.log"
   local input_file="foo.in"
   local output_file="bazel-genfiles/foo.txt"
-  local flags="--experimental_local_disk_cache_path=$cache --experimental_local_disk_cache"
+  local flags="--build_cache=$cache"
 
   rm -rf $cache
   mkdir $cache