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.

PiperOrigin-RevId: 554477775
Change-Id: Ibef994e432764c260a3cab821ca6583c231c5b50

(cherry picked from commit f17b280)
Signed-off-by: Brentley Jones <[email protected]>
  • Loading branch information
tylerwilliams authored and brentleyjones committed Aug 7, 2023
1 parent b9b4713 commit 5a5b6e0
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -443,14 +444,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -57,10 +60,17 @@ public class DiskCacheClient implements RemoteCacheClient {
*/
public DiskCacheClient(
Path root, boolean verifyDownloads, boolean checkActionResult, DigestUtil digestUtil) {
this.root = root;
this.verifyDownloads = verifyDownloads;
this.checkActionResult = checkActionResult;
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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 5a5b6e0

Please sign in to comment.