Flip --remote_cache_async.

Moves uploads to a disk or remote cache to the background by default. Some actions (specifically, those that modify spawn outputs after execution) are opted out, and still upload in a blocking manner. See ActionExecutionMetadata#mayModifySpawnOutputsAfterExecution and overrides.

This is technically not an incompatible change because (in the absence of bugs) it doesn't affect user-visible behavior: asynchronous uploads cannot affect the behavior of hermetic actions.

Fixes bazelbuild#21578.

RELNOTES: Uploading local action results to a disk or remote cache now occurs in the background whenever possible, potentially unblocking the execution of followup actions. Set `--noremote_cache_async` to revert to the previous behavior.
PiperOrigin-RevId: 670152543
Change-Id: Ibf6b36f723945b2534b61e228d440fe71a6019ac

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

@@ -102,13 +102,16 @@ public final class RemoteOptions extends CommonRemoteOptions {
   public PathFragment remoteCaptureCorruptedOutputs;
 
   @Option(
-      name = "experimental_remote_cache_async",
-      defaultValue = "false",
+      name = "remote_cache_async",
+      oldName = "experimental_remote_cache_async",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.REMOTE,
       effectTags = {OptionEffectTag.UNKNOWN},
       help =
-          "If true, remote cache I/O will happen in the background instead of taking place as the"
-              + " part of a spawn.")
+          "If true, uploading of action results to a disk or remote cache will happen in the"
+              + " background instead of blocking the completion of an action. Some actions are"
+              + " incompatible with background uploads, and may still block even when this flag is"
+              + " set.")
   public boolean remoteCacheAsync;
 
   @Option(

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

@@ -1650,7 +1650,7 @@ public void uploadOutputs_uploadDirectory_works() throws Exception {
 
     // act
     UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     // assert
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1697,7 +1697,7 @@ public void uploadOutputs_uploadEmptyDirectory_works() throws Exception {
 
     // act
     UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     // assert
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1771,7 +1771,7 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception {
 
     // act
     UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     // assert
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1807,7 +1807,7 @@ private void doUploadDanglingSymlink(PathFragment targetPath) throws Exception {
 
     // act
     UploadManifest manifest = service.buildUploadManifest(action, spawnResult);
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     // assert
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -1859,7 +1859,7 @@ public void uploadOutputs_emptyOutputs_doNotPerformUpload() throws Exception {
             .build();
 
     // act
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     // assert
     assertThat(
@@ -1885,7 +1885,7 @@ public void uploadOutputs_uploadFails_printWarning() throws Exception {
         .when(cache)
         .uploadActionResult(any(), any(), any());
 
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     assertThat(eventHandler.getEvents()).hasSize(1);
     Event evt = eventHandler.getEvents().get(0);
@@ -1910,7 +1910,7 @@ public void uploadOutputs_firesUploadEvents() throws Exception {
             .setRunnerName("test")
             .build();
 
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     assertThat(eventHandler.getPosts())
         .containsAtLeast(
@@ -1937,7 +1937,7 @@ public void uploadOutputs_missingMandatoryOutputs_dontUpload() throws Exception
             .setRunnerName("test")
             .build();
 
-    service.uploadOutputs(action, spawnResult, () -> {});
+    uploadOutputsAndWait(service, action, spawnResult);
 
     // assert
     assertThat(cache.getNumFindMissingDigests()).isEmpty();
@@ -2578,4 +2578,11 @@ private void createOutputDirectories(Spawn spawn) throws IOException {
       dir.createDirectoryAndParents();
     }
   }
+
+  private static void uploadOutputsAndWait(
+      RemoteExecutionService service, RemoteAction action, SpawnResult result) throws Exception {
+    SettableFuture<Void> future = SettableFuture.create();
+    service.uploadOutputs(action, result, () -> future.set(null));
+    future.get();
+  }
 }