Skip to content

Commit

Permalink
Separate new-style-hash content in DiskCache
Browse files Browse the repository at this point in the history
DiskCache always stores files in /root/{cas/ac}/digestHash. This change keeps the current behavior, but for new style digest functions inserts a directory between /root/ and {cas/ac} to disambiguate the digest function type.

This prevents issues that could theoretically happen if there were hash collisions between two digest functions sharing the same cache directory.

Closes bazelbuild#19073.

PiperOrigin-RevId: 554477775
Change-Id: Ibef994e432764c260a3cab821ca6583c231c5b50
tylerwilliams authored and copybara-github committed Aug 7, 2023

Verified

This commit was signed with the committer’s verified signature.
joshka Josh McKinney
1 parent 15e7ed1 commit f17b280
Showing 5 changed files with 23 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.remote;

import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction;
import static com.google.devtools.build.lib.remote.util.RxFutures.toCompletable;
import static com.google.devtools.build.lib.remote.util.RxFutures.toListenableFuture;
import static com.google.devtools.build.lib.remote.util.RxFutures.toSingle;
@@ -473,14 +474,6 @@ private static class PathConverterImpl implements PathConverter {
this.localPaths = localPaths.build();
}

private static boolean isOldStyleDigestFunction(DigestFunction.Value digestFunction) {
// Old-style digest functions (SHA256, etc) are distinguishable by the length
// of their hash alone and do not require extra specification, but newer
// digest functions (which may have the same length hashes as the older
// functions!) must be explicitly specified in the upload resource name.
return digestFunction.getNumber() <= 7;
}

@Override
@Nullable
public String apply(Path path) {
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction;
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
import static com.google.devtools.build.lib.remote.util.Utils.waitForBulkTransfer;
import static java.lang.String.format;
@@ -179,20 +180,12 @@ public ListenableFuture<Void> uploadBlobAsync(
MoreExecutors.directExecutor());
}

private boolean isOldStyleDigestFunction() {
// Old-style digest functions (SHA256, etc) are distinguishable by the length
// of their hash alone and do not require extra specification, but newer
// digest functions (which may have the same length hashes as the older
// functions!) must be explicitly specified in the upload resource name.
return digestFunction.getNumber() <= 7;
}

private String buildUploadResourceName(
String instanceName, UUID uuid, Digest digest, boolean compressed) {

String resourceName;

if (isOldStyleDigestFunction()) {
if (isOldStyleDigestFunction(digestFunction)) {
String template =
compressed ? "uploads/%s/compressed-blobs/zstd/%s/%d" : "uploads/%s/blobs/%s/%d";
resourceName = format(template, uuid, digest.getHash(), digest.getSizeBytes());
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.remote;

import static com.google.common.base.Strings.isNullOrEmpty;
import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction;

import build.bazel.remote.execution.v2.ActionCacheGrpc;
import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheFutureStub;
@@ -354,14 +355,6 @@ private ListenableFuture<Void> downloadBlob(
MoreExecutors.directExecutor());
}

private static boolean isOldStyleDigestFunction(DigestFunction.Value digestFunction) {
// Old-style digest functions (SHA256, etc) are distinguishable by the length
// of their hash alone and do not require extra specification, but newer
// digest functions (which may have the same length hashes as the older
// functions!) must be explicitly specified in the upload resource name.
return digestFunction.getNumber() <= 7;
}

public static String getResourceName(
String instanceName, Digest digest, boolean compressed, DigestFunction.Value digestFunction) {
String resourceName = "";
Original file line number Diff line number Diff line change
@@ -13,10 +13,13 @@
// limitations under the License.
package com.google.devtools.build.lib.remote.disk;

import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction;

import build.bazel.remote.execution.v2.ActionResult;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.Directory;
import build.bazel.remote.execution.v2.Tree;
import com.google.common.base.Ascii;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.ByteStreams;
import com.google.common.util.concurrent.Futures;
@@ -53,9 +56,16 @@ public class DiskCacheClient implements RemoteCacheClient {
* digest used to index that file.
*/
public DiskCacheClient(Path root, boolean verifyDownloads, DigestUtil digestUtil) {
this.root = root;
this.verifyDownloads = verifyDownloads;
this.digestUtil = digestUtil;

if (isOldStyleDigestFunction(digestUtil.getDigestFunction())) {
this.root = root;
} else {
this.root =
root.getChild(
Ascii.toLowerCase(digestUtil.getDigestFunction().getValueDescriptor().getName()));
}
}

/** Returns {@code true} if the provided {@code key} is stored in the CAS. */
Original file line number Diff line number Diff line change
@@ -131,4 +131,12 @@ public static String toString(Digest digest) {
public static byte[] toBinaryDigest(Digest digest) {
return HashCode.fromString(digest.getHash()).asBytes();
}

public static boolean isOldStyleDigestFunction(DigestFunction.Value digestFunction) {
// Old-style digest functions (SHA256, etc) are distinguishable by the length
// of their hash alone and do not require extra specification, but newer
// digest functions (which may have the same length hashes as the older
// functions!) must be explicitly specified in the upload resource name.
return digestFunction.getNumber() <= 7;
}
}

0 comments on commit f17b280

Please sign in to comment.