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

Ensure disk cache root exists #19202

Closed

Conversation

tylerwilliams
Copy link
Contributor

@tylerwilliams tylerwilliams commented Aug 8, 2023

The disk cache root directory was previously being manually created in two places before creating the disk cache client. This CL instead ensures that the disk cache root directory is created when createDiskCache() is called, and simplifies the other callsites.

This fixes an issue where the disk cache directory might not exist if using --digest_function=BLAKE3.

@tylerwilliams tylerwilliams requested a review from a team as a code owner August 8, 2023 19:57
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Aug 8, 2023
@tylerwilliams tylerwilliams force-pushed the b3_disk_naming2 branch 2 times, most recently from fb75f74 to afca977 Compare August 8, 2023 22:44
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 8, 2023
@BalestraPatrick
Copy link
Member

I tested this change (after manually cherry-picking it into release-.6.40 and fixing conflicts) and it fixed the issue we were seeing. Thank you!

@iancha1992
Copy link
Member

@bazel-io fork 6.4.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 9, 2023
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Aug 10, 2023
@brentleyjones brentleyjones deleted the b3_disk_naming2 branch August 10, 2023 13:10
iancha1992 pushed a commit to iancha1992/bazel that referenced this pull request Aug 10, 2023
The disk cache root directory was previously being manually created in two places before creating the disk cache client. This CL instead ensures that the disk cache root directory is created when createDiskCache() is called, and simplifies the other callsites.

This fixes an issue where the disk cache directory might not exist if using --digest_function=BLAKE3.

Closes bazelbuild#19202.

PiperOrigin-RevId: 555429326
Change-Id: Ifcfa8c686df30c03c9fca24be860b2db780a6374
@iancha1992
Copy link
Member

iancha1992 commented Aug 10, 2023

@tylerwilliams @brentleyjones @coeuvre @BalestraPatrick

When cherry-picking this to release-6.4.0, there were some conflicts.

  1. src/main/java/com/google/devtools/build/lib/remote/RemoteCacheClientFactory.java

createDiskCache returns return new DiskCacheClient(cacheDir, verifyDownloads, checkActionResult, digestUtil) in the release-6.4.0 branch. But it's return new DiskCacheClient(cacheDir, verifyDownloads, digestUtil) in the master branch.

RemoteCacheClient httpCache = createHttp(options, cred, authAndTlsOptions, digestUtil); is in the release-6.4.0 branch. but it is RemoteCacheClient httpCache = createHttp(options, cred, authAndTlsOptions, digestUtil, retrier); in the master branch.

  1. src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java
    In the release-6.4.0 branch currently,
public DiskCacheClient(
      Path root, boolean verifyDownloads, boolean checkActionResult, DigestUtil digestUtil) 

In the master branch currently,

public DiskCacheClient(Path root, boolean verifyDownloads, DigestUtil digestUtil)

It does seem small conflicts. But if there's any commit you want me to push in order to resolve these conflicts, please let me know so we can cherry-pick this for our release-6.4.0. Thanks.

cc: @bazelbuild/triage

@brentleyjones
Copy link
Contributor

I'll submit a PR.

@brentleyjones
Copy link
Contributor

#19225

@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants