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

improve --experimental_local_disk_cache #4870

Closed
buchgr opened this issue Mar 19, 2018 · 12 comments
Closed

improve --experimental_local_disk_cache #4870

buchgr opened this issue Mar 19, 2018 · 12 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request

Comments

@buchgr
Copy link
Contributor

buchgr commented Mar 19, 2018

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

@davido
Copy link
Contributor

davido commented Mar 19, 2018

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).

@ittaiz
Copy link
Member

ittaiz commented Mar 19, 2018 via email

@buchgr
Copy link
Contributor Author

buchgr commented Mar 19, 2018

@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.

@buchgr
Copy link
Contributor Author

buchgr commented Mar 19, 2018

@davido would you want to work on this? 😃

@davido
Copy link
Contributor

davido commented Mar 19, 2018

@buchgr Yes, feel free to assign it to me.

davido added a commit to davido/bazel that referenced this issue Mar 19, 2018
Fixes: bazelbuild#4870.

Test Plan:

  $ bazel test src/test/shell/bazel:local_action_cache_test
davido added a commit to davido/bazel that referenced this issue Mar 19, 2018
Fixes: bazelbuild#4870.

Test Plan:

  $ bazel test src/test/shell/bazel:local_action_cache_test
@hlopko hlopko added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Mar 21, 2018
davido added a commit to davido/bazel that referenced this issue Mar 22, 2018
Fixes: bazelbuild#4870.

Test Plan:

  $ bazel test src/test/shell/bazel:local_action_cache_test
davido added a commit to davido/bazel that referenced this issue Mar 22, 2018
Fixes: bazelbuild#4870.

Test Plan:

  $ bazel test src/test/shell/bazel:build_cache_test
@promiseofcake
Copy link

Should we also have it respect the no-cache options as the remote-cache has? exclude-specific-targets-from-using-the-remote-cache?

@RNabel
Copy link
Contributor

RNabel commented Apr 30, 2018

Is there a timeline for this feature?

@buchgr
Copy link
Contributor Author

buchgr commented Apr 30, 2018

@promiseofcake yes we should.

@RNabel we love contributions :-)

@RNabel
Copy link
Contributor

RNabel commented Apr 30, 2018

@davido There are a few of your commits referenced here -- are you still working on this?

@davido
Copy link
Contributor

davido commented May 1, 2018

@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 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 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

@buchgr
Copy link
Contributor Author

buchgr commented May 2, 2018

@davido's changes have just been merged ... yay! 4ee7f11

I ll open another issue for discussing garbage collection.

@buchgr
Copy link
Contributor Author

buchgr commented May 2, 2018

#5139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants