Address comments bazelbuild#3

Signed-off-by: Luis Fernando Pino Duque <luis@engflow.com>
lfpino · Jul 8, 2022 · df6de24 · df6de24
1 parent fdf3b25
commit df6de24
Showing 1 changed file with 33 additions and 23 deletions.
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
@@ -1067,13 +1067,23 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
     }
 
     ImmutableList<ListenableFuture<FileMetadata>> downloads = downloadsBuilder.build();
-    ImmutableList<ListenableFuture<FileMetadata>> forcedDownloads = forcedDownloadsBuilder.build();
+    try (SilentCloseable c = Profiler.instance().profile("Remote.download")) {
+      waitForBulkTransfer(downloads, /* cancelRemainingOnInterrupt= */ true);
+    } catch (Exception e) {
+      // TODO(bazel-team): Consider adding better case-by-case exception handling instead of just rethrowing
+      captureCorruptedOutputs(e);
+      deletePartialDownloadedOutputs(result, tmpOutErr, e);
+      throw e;
+    }
 
-    try {
-      waitForDownloads(forcedDownloads, result, tmpOutErr, "Remote.download");
-      waitForDownloads(forcedDownloads, result, tmpOutErr, "Remote.forcedDownload");
-    } catch (BulkTransferException | InterruptedException | ExecException e) {
+    ImmutableList<ListenableFuture<FileMetadata>> forcedDownloads = forcedDownloadsBuilder.build();
+    // TODO(bazel-team): Unify this block with the equivalent block above.
+    try (SilentCloseable c = Profiler.instance().profile("Remote.forcedDownload")) {
+      waitForBulkTransfer(forcedDownloads, /* cancelRemainingOnInterrupt= */ true);
+    } catch (Exception e) {
       // TODO(bazel-team): Consider adding better case-by-case exception handling instead of just rethrowing
+      captureCorruptedOutputs(e);
+      deletePartialDownloadedOutputs(result, tmpOutErr, e);
       throw e;
     }
 
@@ -1088,13 +1098,6 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
     tmpOutErr.clearOut();
     tmpOutErr.clearErr();
 
-    // TODO(bazel-team): We should unify this if-block to rely on downloadOutputs above but, as of 2022-07-05,
-    //  downloadOuputs' semantics isn't exactly the same as build-without-the-bytes which is necessary for using
-    //  remoteDownloadRegex.
-    if (!forcedDownloads.isEmpty()) {
-      moveOutputsToFinalLocation(forcedDownloads);
-    }
-
     if (downloadOutputs) {
       moveOutputsToFinalLocation(downloads);
 
@@ -1112,6 +1115,13 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
       // might not be supported on all platforms
       createSymlinks(symlinks);
     } else {
+      // TODO(bazel-team): We should unify this if-block to rely on downloadOutputs above but, as of 2022-07-05,
+      //  downloadOuputs' semantics isn't exactly the same as build-without-the-bytes which is necessary for using
+      //  remoteDownloadRegex.
+      if (!forcedDownloads.isEmpty()) {
+        moveOutputsToFinalLocation(forcedDownloads);
+      }
+
       ActionInput inMemoryOutput = null;
       Digest inMemoryOutputDigest = null;
       PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.getSpawn());
@@ -1149,17 +1159,17 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
     return null;
   }
 
-  private void waitForDownloads(ImmutableList<ListenableFuture<FileMetadata>> downloads,
-                   RemoteActionResult result, FileOutErr tmpOutErr, String profileDescriptor)
-          throws BulkTransferException, InterruptedException, ExecException {
-    try (SilentCloseable c = Profiler.instance().profile(profileDescriptor)) {
-      waitForBulkTransfer(downloads, /* cancelRemainingOnInterrupt= */ true);
-    } catch (BulkTransferException | InterruptedException e) {
-      captureCorruptedOutputs(e);
-      deletePartialDownloadedOutputs(result, tmpOutErr, e);
-      throw e;
-    }
-  }
+//  private void waitForDownloads(ImmutableList<ListenableFuture<FileMetadata>> downloads,
+//                   RemoteActionResult result, FileOutErr tmpOutErr, String profileDescriptor)
+//          throws Exception {
+//    try (SilentCloseable c = Profiler.instance().profile(profileDescriptor)) {
+//      waitForBulkTransfer(downloads, /* cancelRemainingOnInterrupt= */ true);
+//    } catch (Exception e) {
+//      captureCorruptedOutputs(e);
+//      deletePartialDownloadedOutputs(result, tmpOutErr, e);
+//      throw e;
+//    }
+//  }
 
   private ImmutableList<ListenableFuture<FileMetadata>> buildFilesToDownload(
           ActionResultMetadata metadata, RemoteAction action) {