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:

Patch that resolved the issue:
diff --git a/site/docs/remote-caching.md b/site/docs/remote-caching.md
index 163f1122f2ad69..7a949d9d271b57 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)
+* [Disk cache](#disk-cache)
 * [Bazel remote execution (in development)](#bazel-remote-execution-in-development)
 
 ## Remote caching overview
@@ -306,6 +307,30 @@ 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
 
+## Disk 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 disk cache
+can be enabled with the following flags.
+
+```
+build --disk_cache=/path/to/build/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.
+
+To have cache hits across different workspaces, the flag 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 8877f98572fb90..df8c312ef54a8f 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
@@ -64,7 +64,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.diskCache != 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 dd4f5a0d04878a..385edf5adf5300 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
@@ -175,25 +175,16 @@ public final class RemoteOptions extends OptionsBase {
   )
   public boolean experimentalRemoteSpawnCache;
 
-  // TODO(davido): Find a better place for this and the next option.
   @Option(
-    name = "experimental_local_disk_cache",
-    defaultValue = "false",
-    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 = "disk_cache",
     defaultValue = "null",
     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 diskCache;
 
   @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..2a408042f1eeba 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
@@ -21,6 +21,7 @@
 import com.google.devtools.build.lib.remote.blobstore.SimpleBlobStore;
 import com.google.devtools.build.lib.remote.blobstore.http.HttpBlobStore;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
 import java.net.URI;
 import java.util.concurrent.TimeUnit;
@@ -34,8 +35,7 @@ public final class SimpleBlobStoreFactory {
 
   private SimpleBlobStoreFactory() {}
 
-  public static SimpleBlobStore createRest(RemoteOptions options, Credentials creds)
-      throws IOException {
+  public static SimpleBlobStore createRest(RemoteOptions options, Credentials creds) {
     try {
       return new HttpBlobStore(
           URI.create(options.remoteHttpCache),
@@ -46,20 +46,22 @@ public static SimpleBlobStore createRest(RemoteOptions options, Credentials cred
     }
   }
 
-  public static SimpleBlobStore createLocalDisk(RemoteOptions options, Path workingDirectory)
+  public static SimpleBlobStore createDiskCache(Path workingDirectory, PathFragment diskCachePath)
       throws IOException {
-    return new OnDiskBlobStore(
-        workingDirectory.getRelative(checkNotNull(options.experimentalLocalDiskCachePath)));
+    Path cacheDir = workingDirectory.getRelative(checkNotNull(diskCachePath));
+    if (!cacheDir.exists()) {
+      cacheDir.createDirectoryAndParents();
+    }
+    return new OnDiskBlobStore(cacheDir);
   }
 
-  public static SimpleBlobStore create(
-      RemoteOptions options, @Nullable Credentials creds, @Nullable Path workingDirectory)
-      throws IOException {
+  public static SimpleBlobStore create(RemoteOptions options, @Nullable Credentials creds,
+      @Nullable Path workingDirectory) throws IOException {
     if (isRestUrlOptions(options)) {
       return createRest(options, creds);
     }
-    if (workingDirectory != null && isLocalDiskCache(options)) {
-      return createLocalDisk(options, workingDirectory);
+    if (workingDirectory != null && isDiskCache(options)) {
+      return createDiskCache(workingDirectory, options.diskCache);
     }
     throw new IllegalArgumentException(
         "Unrecognized concurrent map RemoteOptions: must specify "
@@ -67,11 +69,11 @@ public static SimpleBlobStore create(
   }
 
   public static boolean isRemoteCacheOptions(RemoteOptions options) {
-    return isRestUrlOptions(options) || isLocalDiskCache(options);
+    return isRestUrlOptions(options) || isDiskCache(options);
   }
 
-  public static boolean isLocalDiskCache(RemoteOptions options) {
-    return options.experimentalLocalDiskCache;
+  public static boolean isDiskCache(RemoteOptions options) {
+    return options.diskCache != null;
   }
 
   private static boolean isRestUrlOptions(RemoteOptions options) {


Test patch:
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index f9c51e0e888afb..bfa6d6cd935614 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -332,9 +332,9 @@ sh_test(
 )
 
 sh_test(
-    name = "local_action_cache_test",
+    name = "disk_cache_test",
     size = "small",
-    srcs = ["local_action_cache_test.sh"],
+    srcs = ["disk_cache_test.sh"],
     data = [":test-deps"],
 )
 
diff --git a/src/test/shell/bazel/local_action_cache_test.sh b/src/test/shell/bazel/disk_cache_test.sh
similarity index 94%
rename from src/test/shell/bazel/local_action_cache_test.sh
rename to src/test/shell/bazel/disk_cache_test.sh
index 60c9c4ecc1abf4..ef224cafefdba7 100755
--- a/src/test/shell/bazel/local_action_cache_test.sh
+++ b/src/test/shell/bazel/disk_cache_test.sh
@@ -14,7 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-# Test the local action cache
+# Test the local disk cache
 #
 
 # Load the test setup defined in the parent directory
@@ -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="--disk_cache=$cache"
 
   rm -rf $cache
   mkdir $cache