Skip to content

Commit

Permalink
Set the digest_function field as part of all relevant gRPC requests
Browse files Browse the repository at this point in the history
In the following PR we extended most of the REv2 *Request messages to
take an explicit digest function:

bazelbuild/remote-apis#235

This eliminates the ambiguity between digest functions that have the
same hash length (e.g., MD5 and MURMUR3, SHA256 and SHA256TREE). This
change extends the REv2 client in Bazel to set the digest_function field
explicitly, so that the server does not need to use any heuristics to
pick a digest function when processing requests.

As we are now going to call DigestUtil.getDigestFunction() far more
frequently, alter DigestUtil to precompute the value upon construction.
  • Loading branch information
EdSchouten committed Mar 14, 2023
1 parent 96d73da commit 1a5e926
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ private int computeMaxMissingBlobsDigestsPerMessage() {
final int overhead =
FindMissingBlobsRequest.newBuilder()
.setInstanceName(options.remoteInstanceName)
.setDigestFunction(digestUtil.getDigestFunction())
.build()
.getSerializedSize();
final int tagSize =
Expand Down Expand Up @@ -185,7 +186,9 @@ public ListenableFuture<ImmutableSet<Digest>> findMissingDigests(
}
// Need to potentially split the digests into multiple requests.
FindMissingBlobsRequest.Builder requestBuilder =
FindMissingBlobsRequest.newBuilder().setInstanceName(options.remoteInstanceName);
FindMissingBlobsRequest.newBuilder()
.setInstanceName(options.remoteInstanceName)
.setDigestFunction(digestUtil.getDigestFunction());
List<ListenableFuture<FindMissingBlobsResponse>> getMissingDigestCalls = new ArrayList<>();
for (Digest digest : digests) {
requestBuilder.addBlobDigests(digest);
Expand Down Expand Up @@ -260,6 +263,7 @@ public ListenableFuture<CachedActionResult> downloadActionResult(
GetActionResultRequest request =
GetActionResultRequest.newBuilder()
.setInstanceName(options.remoteInstanceName)
.setDigestFunction(digestUtil.getDigestFunction())
.setActionDigest(actionKey.getDigest())
.setInlineStderr(inlineOutErr)
.setInlineStdout(inlineOutErr)
Expand Down Expand Up @@ -289,6 +293,7 @@ public ListenableFuture<Void> uploadActionResult(
.updateActionResult(
UpdateActionResultRequest.newBuilder()
.setInstanceName(options.remoteInstanceName)
.setDigestFunction(digestUtil.getDigestFunction())
.setActionDigest(actionKey.getDigest())
.setActionResult(actionResult)
.build())),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1486,6 +1486,7 @@ public RemoteActionResult executeRemotely(
ExecuteRequest.Builder requestBuilder =
ExecuteRequest.newBuilder()
.setInstanceName(remoteOptions.remoteInstanceName)
.setDigestFunction(digestUtil.getDigestFunction())
.setActionDigest(action.getActionKey().getDigest())
.setSkipCacheLookup(!acceptCachedResult);
if (remoteOptions.remoteResultCachePriority != 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ public ExecutionResult execute(
ExecuteRequest.newBuilder()
.setActionDigest(actionDigest)
.setInstanceName(remoteInstanceName)
.setDigestFunction(digestUtil.getDigestFunction())
.setSkipCacheLookup(!acceptCached)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@
public class DigestUtil {
private final XattrProvider xattrProvider;
private final DigestHashFunction hashFn;
private final DigestFunction.Value digestFunction;

public DigestUtil(XattrProvider xattrProvider, DigestHashFunction hashFn) {
this.xattrProvider = xattrProvider;
this.hashFn = hashFn;
this.digestFunction = getDigestFunctionFromHashFunction(hashFn);
}

/** Returns the currently used digest function. */
public DigestFunction.Value getDigestFunction() {
private static DigestFunction.Value getDigestFunctionFromHashFunction(DigestHashFunction hashFn) {
for (String name : hashFn.getNames()) {
try {
return DigestFunction.Value.valueOf(name);
Expand All @@ -53,6 +54,12 @@ public DigestFunction.Value getDigestFunction() {
return DigestFunction.Value.UNKNOWN;
}


/** Returns the currently used digest function. */
public DigestFunction.Value getDigestFunction() {
return digestFunction;
}

public Digest compute(byte[] blob) {
return buildDigest(hashFn.getHashFunction().hashBytes(blob).toString(), blob.length);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc.ContentAddressableStorageImplBase;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.DigestFunction;
import build.bazel.remote.execution.v2.Directory;
import build.bazel.remote.execution.v2.DirectoryNode;
import build.bazel.remote.execution.v2.FileNode;
Expand Down Expand Up @@ -798,6 +799,7 @@ public void updateActionResult(
assertThat(request)
.isEqualTo(
UpdateActionResultRequest.newBuilder()
.setDigestFunction(DigestFunction.Value.SHA256)
.setActionDigest(fooDigest)
.setActionResult(result)
.build());
Expand Down
Loading

0 comments on commit 1a5e926

Please sign in to comment.