Skip to content

Commit

Permalink
Flip --remote_cache_async.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tjgq authored and copybara-github committed Sep 2, 2024
1 parent 100438c commit 0a0d877
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1859,7 +1859,7 @@ public void uploadOutputs_emptyOutputs_doNotPerformUpload() throws Exception {
.build();

// act
service.uploadOutputs(action, spawnResult, () -> {});
uploadOutputsAndWait(service, action, spawnResult);

// assert
assertThat(
Expand All @@ -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);
Expand All @@ -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(
Expand All @@ -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();
Expand Down Expand Up @@ -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();
}
}

0 comments on commit 0a0d877

Please sign in to comment.