Add output name to CacheNotFoundException

to make it easier to debug errors related to remote cache eviction. Fixes #10025. PiperOrigin-RevId: 563361764 Change-Id: I2c29cd521c7ec1293f770b28a5cdcdf356750396
bazelbuild · Sep 7, 2023 · d846d7a · d846d7a · brentleyjones · Sep 7, 2023
1 parent 693ffa6
commit d846d7a
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 20 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
@@ -92,6 +92,7 @@ protected ListenableFuture<Void> doDownloadFile(
 
     return remoteCache.downloadFile(
         context,
+        execPath.getPathString(),
         tempPath,
         digest,
         new RemoteCache.DownloadProgressReporter(

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -33,6 +33,7 @@
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
 import com.google.devtools.build.lib.exec.SpawnProgressEvent;
+import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
 import com.google.devtools.build.lib.remote.common.LazyFileOutputStream;
 import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
 import com.google.devtools.build.lib.remote.common.ProgressStatusListener;
@@ -89,9 +90,7 @@ public class RemoteCache extends AbstractReferenceCounted {
   protected final DigestUtil digestUtil;
 
   public RemoteCache(
-      RemoteCacheClient cacheProtocol,
-      RemoteOptions options,
-      DigestUtil digestUtil) {
+      RemoteCacheClient cacheProtocol, RemoteOptions options, DigestUtil digestUtil) {
     this.cacheProtocol = cacheProtocol;
     this.options = options;
     this.digestUtil = digestUtil;
@@ -198,21 +197,34 @@ protected ListenableFuture<Void> uploadBlob(
     return RxFutures.toListenableFuture(upload);
   }
 
+  public ListenableFuture<byte[]> downloadBlob(
+      RemoteActionExecutionContext context, Digest digest) {
+    return downloadBlob(context, /* blobName= */ "", digest);
+  }
+
   /**
    * Downloads a blob with content hash {@code digest} and stores its content in memory.
    *
    * @return a future that completes after the download completes (succeeds / fails). If successful,
    *     the content is stored in the future's {@code byte[]}.
    */
   public ListenableFuture<byte[]> downloadBlob(
-      RemoteActionExecutionContext context, Digest digest) {
+      RemoteActionExecutionContext context, String blobName, Digest digest) {
     if (digest.getSizeBytes() == 0) {
       return EMPTY_BYTES;
     }
     ByteArrayOutputStream bOut = new ByteArrayOutputStream((int) digest.getSizeBytes());
+    var download = downloadBlob(context, blobName, digest, bOut);
     SettableFuture<byte[]> outerF = SettableFuture.create();
+    outerF.addListener(
+        () -> {
+          if (outerF.isCancelled()) {
+            download.cancel(/* mayInterruptIfRunning= */ true);
+          }
+        },
+        directExecutor());
     Futures.addCallback(
-        cacheProtocol.downloadBlob(context, digest, bOut),
+        download,
         new FutureCallback<Void>() {
           @Override
           public void onSuccess(Void aVoid) {
@@ -234,12 +246,37 @@ public void onFailure(Throwable t) {
   }
 
   private ListenableFuture<Void> downloadBlob(
-      RemoteActionExecutionContext context, Digest digest, OutputStream out) {
+      RemoteActionExecutionContext context, String blobName, Digest digest, OutputStream out) {
     if (digest.getSizeBytes() == 0) {
       return COMPLETED_SUCCESS;
     }
+    var download = cacheProtocol.downloadBlob(context, digest, out);
+    SettableFuture<Void> future = SettableFuture.create();
+    future.addListener(
+        () -> {
+          if (future.isCancelled()) {
+            download.cancel(/* mayInterruptIfRunning= */ true);
+          }
+        },
+        directExecutor());
+    Futures.addCallback(
+        download,
+        new FutureCallback<Void>() {
+          @Override
+          public void onSuccess(Void result) {
+            future.set(result);
+          }
 
-    return cacheProtocol.downloadBlob(context, digest, out);
+          @Override
+          public void onFailure(Throwable t) {
+            if (t instanceof CacheNotFoundException) {
+              ((CacheNotFoundException) t).setFilename(blobName);
+            }
+            future.setException(t);
+          }
+        },
+        directExecutor());
+    return future;
   }
 
   /** A reporter that reports download progresses. */
@@ -312,6 +349,13 @@ public ListenableFuture<Void> downloadFile(
       throws IOException {
     SettableFuture<Void> outerF = SettableFuture.create();
     ListenableFuture<Void> f = downloadFile(context, localPath, digest, reporter);
+    outerF.addListener(
+        () -> {
+          if (outerF.isCancelled()) {
+            f.cancel(/* mayInterruptIfRunning= */ true);
+          }
+        },
+        directExecutor());
     Futures.addCallback(
         f,
         new FutureCallback<Void>() {
@@ -322,7 +366,10 @@ public void onSuccess(Void unused) {
 
           @Override
           public void onFailure(Throwable throwable) {
-            if (throwable instanceof OutputDigestMismatchException) {
+            if (throwable instanceof CacheNotFoundException) {
+              var cacheNotFoundException = (CacheNotFoundException) throwable;
+              cacheNotFoundException.setFilename(outputPath);
+            } else if (throwable instanceof OutputDigestMismatchException) {
               OutputDigestMismatchException e = ((OutputDigestMismatchException) throwable);
               e.setOutputPath(outputPath);
               e.setLocalPath(localPath);
@@ -338,11 +385,16 @@ public void onFailure(Throwable throwable) {
   /** Downloads a file (that is not a directory). The content is fetched from the digest. */
   public ListenableFuture<Void> downloadFile(
       RemoteActionExecutionContext context, Path path, Digest digest) throws IOException {
-    return downloadFile(context, path, digest, new DownloadProgressReporter(NO_ACTION, "", 0));
+    return downloadFile(
+        context,
+        path.getPathString(),
+        path,
+        digest,
+        new DownloadProgressReporter(NO_ACTION, "", 0));
   }
 
   /** Downloads a file (that is not a directory). The content is fetched from the digest. */
-  public ListenableFuture<Void> downloadFile(
+  private ListenableFuture<Void> downloadFile(
       RemoteActionExecutionContext context,
       Path path,
       Digest digest,
@@ -406,7 +458,12 @@ public final List<ListenableFuture<Void>> downloadOutErr(
         downloads.add(Futures.immediateFailedFuture(e));
       }
     } else if (result.hasStdoutDigest()) {
-      downloads.add(downloadBlob(context, result.getStdoutDigest(), outErr.getOutputStream()));
+      downloads.add(
+          downloadBlob(
+              context,
+              /* blobName= */ "<stdout>",
+              result.getStdoutDigest(),
+              outErr.getOutputStream()));
     }
     if (!result.getStderrRaw().isEmpty()) {
       try {
@@ -416,7 +473,12 @@ public final List<ListenableFuture<Void>> downloadOutErr(
         downloads.add(Futures.immediateFailedFuture(e));
       }
     } else if (result.hasStderrDigest()) {
-      downloads.add(downloadBlob(context, result.getStderrDigest(), outErr.getErrorStream()));
+      downloads.add(
+          downloadBlob(
+              context,
+              /* blobName= */ "<stderr>",
+              result.getStderrDigest(),
+              outErr.getErrorStream()));
     }
     return downloads;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -972,10 +972,11 @@ ActionResultMetadata parseActionResultMetadata(
     Map<Path, ListenableFuture<Tree>> dirMetadataDownloads =
         Maps.newHashMapWithExpectedSize(result.getOutputDirectoriesCount());
     for (OutputDirectory dir : result.getOutputDirectories()) {
+      var outputPath = encodeBytestringUtf8(dir.getPath());
       dirMetadataDownloads.put(
-          remotePathResolver.outputPathToLocalPath(encodeBytestringUtf8(dir.getPath())),
+          remotePathResolver.outputPathToLocalPath(outputPath),
           Futures.transformAsync(
-              remoteCache.downloadBlob(context, dir.getTreeDigest()),
+              remoteCache.downloadBlob(context, outputPath, dir.getTreeDigest()),
               (treeBytes) ->
                   immediateFuture(Tree.parseFrom(treeBytes, ExtensionRegistry.getEmptyRegistry())),
               directExecutor()));
@@ -1117,7 +1118,8 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
         if (isInMemoryOutputFile) {
           downloadsBuilder.add(
               transform(
-                  remoteCache.downloadBlob(context, file.digest()),
+                  remoteCache.downloadBlob(
+                      context, inMemoryOutputPath.getPathString(), file.digest()),
                   data -> {
                     inMemoryOutputData.set(data);
                     return null;

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
@@ -86,14 +86,18 @@ private ExecutionResult downloadOutErr(RemoteActionExecutionContext context, Act
       if (!result.getStdoutRaw().isEmpty()) {
         stdout = result.getStdoutRaw().toByteArray();
       } else if (result.hasStdoutDigest()) {
-        stdout = Utils.getFromFuture(remoteCache.downloadBlob(context, result.getStdoutDigest()));
+        stdout =
+            Utils.getFromFuture(
+                remoteCache.downloadBlob(context, "<stdout>", result.getStdoutDigest()));
       }
 
       byte[] stderr = new byte[0];
       if (!result.getStderrRaw().isEmpty()) {
         stderr = result.getStderrRaw().toByteArray();
       } else if (result.hasStderrDigest()) {
-        stderr = Utils.getFromFuture(remoteCache.downloadBlob(context, result.getStderrDigest()));
+        stderr =
+            Utils.getFromFuture(
+                remoteCache.downloadBlob(context, "<stderr>", result.getStderrDigest()));
       }
 
       return new ExecutionResult(result.getExitCode(), stdout, stderr);
@@ -138,7 +142,7 @@ public ExecutionResult execute(
             platform,
             timeout,
             acceptCached,
-            /*salt=*/ null);
+            /* salt= */ null);
     Digest actionDigest = digestUtil.compute(action);
     ActionKey actionKey = new ActionKey(actionDigest);
     CachedActionResult cachedActionResult;
@@ -158,7 +162,7 @@ public ExecutionResult execute(
         additionalInputs.put(actionDigest, action);
         additionalInputs.put(commandHash, command);
 
-        remoteCache.ensureInputsPresent(context, merkleTree, additionalInputs, /*force=*/ true);
+        remoteCache.ensureInputsPresent(context, merkleTree, additionalInputs, /* force= */ true);
       }
 
       try (SilentCloseable c =

diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
@@ -24,6 +24,8 @@ java_library(
     name = "cache_not_found_exception",
     srcs = ["CacheNotFoundException.java"],
     deps = [
+        "//third_party:guava",
+        "//third_party:jsr305",
         "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto",
     ],
 )

diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/CacheNotFoundException.java b/src/main/java/com/google/devtools/build/lib/remote/common/CacheNotFoundException.java
@@ -15,21 +15,37 @@
 package com.google.devtools.build.lib.remote.common;
 
 import build.bazel.remote.execution.v2.Digest;
+import com.google.common.base.Strings;
 import java.io.IOException;
+import javax.annotation.Nullable;
 
 /**
  * An exception to indicate cache misses. TODO(olaola): have a class of checked
  * RemoteCacheExceptions.
  */
 public final class CacheNotFoundException extends IOException {
   private final Digest missingDigest;
+  @Nullable private String filename;
 
   public CacheNotFoundException(Digest missingDigest) {
-    super("Missing digest: " + missingDigest.getHash() + "/" + missingDigest.getSizeBytes());
     this.missingDigest = missingDigest;
   }
 
+  public void setFilename(@Nullable String filename) {
+    this.filename = filename;
+  }
+
   public Digest getMissingDigest() {
     return missingDigest;
   }
+
+  @Override
+  public String getMessage() {
+    String message =
+        "Missing digest: " + missingDigest.getHash() + "/" + missingDigest.getSizeBytes();
+    if (!Strings.isNullOrEmpty(filename)) {
+      message += " for " + filename;
+    }
+    return message;
+  }
 }