Skip to content

Commit 1a5e926

Browse files
committed
Set the digest_function field as part of all relevant gRPC requests
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.
1 parent 96d73da commit 1a5e926

File tree

6 files changed

+194
-16
lines changed

6 files changed

+194
-16
lines changed

src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ private int computeMaxMissingBlobsDigestsPerMessage() {
117117
final int overhead =
118118
FindMissingBlobsRequest.newBuilder()
119119
.setInstanceName(options.remoteInstanceName)
120+
.setDigestFunction(digestUtil.getDigestFunction())
120121
.build()
121122
.getSerializedSize();
122123
final int tagSize =
@@ -185,7 +186,9 @@ public ListenableFuture<ImmutableSet<Digest>> findMissingDigests(
185186
}
186187
// Need to potentially split the digests into multiple requests.
187188
FindMissingBlobsRequest.Builder requestBuilder =
188-
FindMissingBlobsRequest.newBuilder().setInstanceName(options.remoteInstanceName);
189+
FindMissingBlobsRequest.newBuilder()
190+
.setInstanceName(options.remoteInstanceName)
191+
.setDigestFunction(digestUtil.getDigestFunction());
189192
List<ListenableFuture<FindMissingBlobsResponse>> getMissingDigestCalls = new ArrayList<>();
190193
for (Digest digest : digests) {
191194
requestBuilder.addBlobDigests(digest);
@@ -260,6 +263,7 @@ public ListenableFuture<CachedActionResult> downloadActionResult(
260263
GetActionResultRequest request =
261264
GetActionResultRequest.newBuilder()
262265
.setInstanceName(options.remoteInstanceName)
266+
.setDigestFunction(digestUtil.getDigestFunction())
263267
.setActionDigest(actionKey.getDigest())
264268
.setInlineStderr(inlineOutErr)
265269
.setInlineStdout(inlineOutErr)
@@ -289,6 +293,7 @@ public ListenableFuture<Void> uploadActionResult(
289293
.updateActionResult(
290294
UpdateActionResultRequest.newBuilder()
291295
.setInstanceName(options.remoteInstanceName)
296+
.setDigestFunction(digestUtil.getDigestFunction())
292297
.setActionDigest(actionKey.getDigest())
293298
.setActionResult(actionResult)
294299
.build())),

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,6 +1486,7 @@ public RemoteActionResult executeRemotely(
14861486
ExecuteRequest.Builder requestBuilder =
14871487
ExecuteRequest.newBuilder()
14881488
.setInstanceName(remoteOptions.remoteInstanceName)
1489+
.setDigestFunction(digestUtil.getDigestFunction())
14891490
.setActionDigest(action.getActionKey().getDigest())
14901491
.setSkipCacheLookup(!acceptCachedResult);
14911492
if (remoteOptions.remoteResultCachePriority != 0) {

src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ public ExecutionResult execute(
167167
ExecuteRequest.newBuilder()
168168
.setActionDigest(actionDigest)
169169
.setInstanceName(remoteInstanceName)
170+
.setDigestFunction(digestUtil.getDigestFunction())
170171
.setSkipCacheLookup(!acceptCached)
171172
.build();
172173

src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,15 @@
3535
public class DigestUtil {
3636
private final XattrProvider xattrProvider;
3737
private final DigestHashFunction hashFn;
38+
private final DigestFunction.Value digestFunction;
3839

3940
public DigestUtil(XattrProvider xattrProvider, DigestHashFunction hashFn) {
4041
this.xattrProvider = xattrProvider;
4142
this.hashFn = hashFn;
43+
this.digestFunction = getDigestFunctionFromHashFunction(hashFn);
4244
}
4345

44-
/** Returns the currently used digest function. */
45-
public DigestFunction.Value getDigestFunction() {
46+
private static DigestFunction.Value getDigestFunctionFromHashFunction(DigestHashFunction hashFn) {
4647
for (String name : hashFn.getNames()) {
4748
try {
4849
return DigestFunction.Value.valueOf(name);
@@ -53,6 +54,12 @@ public DigestFunction.Value getDigestFunction() {
5354
return DigestFunction.Value.UNKNOWN;
5455
}
5556

57+
58+
/** Returns the currently used digest function. */
59+
public DigestFunction.Value getDigestFunction() {
60+
return digestFunction;
61+
}
62+
5663
public Digest compute(byte[] blob) {
5764
return buildDigest(hashFn.getHashFunction().hashBytes(blob).toString(), blob.length);
5865
}

src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import build.bazel.remote.execution.v2.Command;
3131
import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc.ContentAddressableStorageImplBase;
3232
import build.bazel.remote.execution.v2.Digest;
33+
import build.bazel.remote.execution.v2.DigestFunction;
3334
import build.bazel.remote.execution.v2.Directory;
3435
import build.bazel.remote.execution.v2.DirectoryNode;
3536
import build.bazel.remote.execution.v2.FileNode;
@@ -798,6 +799,7 @@ public void updateActionResult(
798799
assertThat(request)
799800
.isEqualTo(
800801
UpdateActionResultRequest.newBuilder()
802+
.setDigestFunction(DigestFunction.Value.SHA256)
801803
.setActionDigest(fooDigest)
802804
.setActionResult(result)
803805
.build());

0 commit comments

Comments
 (0)