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

Bazel: It should be possible to disable the disk cache #8802

Closed
nnobelis opened this issue Jun 27, 2024 · 8 comments
Closed

Bazel: It should be possible to disable the disk cache #8802

nnobelis opened this issue Jun 27, 2024 · 8 comments
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements

Comments

@nnobelis
Copy link
Member

The user can configure the usage of a disk cache in the .bazelrc file:

common --disk_cache=/home/dockeruser/.cache/bazel_disk_cache/

The pointed directory can be outside of the repository due to a specific environment.

When the pointed directory does not exist, ORT crashes on any bazel command, even bazel --version:

Exception in thread "main" java.io.IOException: Running 'bazel version' in '/test/' failed with exit code 2:
ERROR: /home/dockeruser/.cache/bazel_disk_cache (Permission denied)
ERROR: Error initializing RemoteModule

The workaround (taken here) is to pass --disk_cache= as extra parameter to the Bazel commands run by ORT.
TBD if it should be configurable or passed every time.

@haikoschol FYI

@nnobelis nnobelis added enhancement Issues that are considered to be enhancements to triage Issues that need triaging labels Jun 27, 2024
@nnobelis nnobelis changed the title Bazel: it should be possible to disable the disk cache Bazel: It should be possible to disable the disk cache Jun 27, 2024
@sschuberth sschuberth added analyzer About the analyzer tool and removed to triage Issues that need triaging labels Jun 27, 2024
@fviernau
Copy link
Member

Instead of adding a parameter, would it be an option that ORT simply takes care of creating the directory in case it does not exist?

@fviernau
Copy link
Member

I've just tried to reproduce the issue, by deleting .cache/bazel_disk_cache inside docker and then run the bazel fun test. The tests succeeds. So, I wonder if the issue description is correct.

Could the problem be a permission issue rather than the non-existance of the dir?

@nnobelis
Copy link
Member Author

and then run the bazel fun test.

But did you test with the .bazelrc configuration described above ? I am not sure the disk cache is enabled by default.

would it be an option that ORT simply takes care of creating the directory in case it does not exist

Creating this directory works but I don't see how ORT can get the name of this directory? By parsing the .bazelrc file?

Don't forget that some projects can use a custom Bazel RC e.g. bazel --bazelrc=my_custom_bazelrc.

@fviernau
Copy link
Member

fviernau commented Jun 28, 2024

But did you test with the .bazelrc configuration described above ? I am not sure the disk cache is enabled by default.

Oh no, I missed that this step is required.

In my view, it could be a good solution to always overwrite any disk cache directory configured in .bazelrc.
I've tested this and it indeed avoids running into the error. (Advantage, no user configuration is required) For example, the following would work even if the disk cache dir specified in .bazelrc does not exist:

mkdir -p ~/.cache/ort/bazel/disk-cache
bazel mod graph --disk_cache ~/.cache/ort/bazel/disk-cache

Maybe a better directory could be choosen.

@fviernau
Copy link
Member

One additional question I just came across is whether the content of the cache dir should be re-used across scans on CI.
We have ~/.ort/cache/analyzer which I believe is normally re-used. In this case (as its a build cache) it may be better to create a new empty temp directory, and use it only for a single ORT analysis process. What do you think ?

@nnobelis
Copy link
Member Author

Yes I agree: better to use an empty temp directory for disk cache.

However I am not sure the commands run by ORT will fill the cache: these are not doing "a real build", no ?

@fviernau
Copy link
Member

fviernau commented Jun 28, 2024

However I am not sure the commands run by ORT will fill the cache: these are not doing "a real build", no ?

I'm not into this implementation deeply, but I do recall from the code review that there may be a build involved.

As I played around anyway, I quickly made a PR to illustrate my thinking. Let's discuss there
whether the approach is feasible. It has the short coming of not re-using any pre-existing build cache,
probably only relevant when run on a dev machine. See: #8811

fviernau added a commit that referenced this issue Jun 28, 2024
When the disk-cache is set via `.bazelrc` to a non-existing directory,
`bazel mod graph` fails with a complaint about permissions, which in
turn makes the analysis fail.

The disk cache is a build cache which seems to target speeding up
builds in case of code changes (such as switching branches) [1]. During
ORT analysis code changes are not in play, so the disk cache seems to
be of little use anyway. Simply disable it to avoid running into that
permission issue.

Fixes: #8802.

[1] https://stackoverflow.com/questions/61282419/bazel-how-outputroot-and-disk-cache-relate-regarding-local-caching

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit that referenced this issue Jun 28, 2024
When the disk-cache is set via `.bazelrc` to a non-existing directory,
`bazel mod graph` fails with a complaint about permissions, which in
turn makes the analysis fail.

The disk cache is a build cache which seems to target speeding up
builds in case of code changes (such as switching branches) [1]. During
ORT analysis code changes are not in play, so the disk cache seems to
be of little use anyway. Simply disable it to avoid running into that
permission issue.

Fixes: #8802.

[1] https://stackoverflow.com/questions/61282419/bazel-how-outputroot-and-disk-cache-relate-regarding-local-caching

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit that referenced this issue Jun 28, 2024
When the disk-cache is set via `.bazelrc` to a non-existing directory,
`bazel mod graph` fails with a complaint about permissions, which in
turn makes the analysis fail.

The disk cache is a build cache which seems to target speeding up
builds in case of code changes (such as switching branches) [1]. During
ORT analysis code changes are not in play, so the disk cache seems to
be of little use anyway. Simply disable it to avoid running into that
permission issue.

Fixes: #8802.

[1] https://stackoverflow.com/questions/61282419/bazel-how-outputroot-and-disk-cache-relate-regarding-local-caching

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit that referenced this issue Jun 28, 2024
When the disk-cache is set via `.bazelrc` to a non-existing directory,
`bazel mod graph` fails with a complaint about permissions, which in
turn makes the analysis fail.

The disk cache is a build cache which seems to target speeding up
builds in case of code changes (such as switching branches) [1]. During
ORT analysis code changes are not in play, so the disk cache seems to
be of little use anyway. Simply disable it to avoid running into that
permission issue.

Fixes: #8802.

[1] https://stackoverflow.com/questions/61282419/bazel-how-outputroot-and-disk-cache-relate-regarding-local-caching

Signed-off-by: Frank Viernau <[email protected]>
@nnobelis
Copy link
Member Author

Thanks for the fix @fviernau!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements
Projects
None yet
Development

No branches or pull requests

3 participants