Skip to content

Commit 68e1924

Browse files
coeuvretjgq
coeuvre
and
tjgq
authored
Also handle remote cache eviction for tree artifacts. (bazelbuild#17601)
Related to bazelbuild#17366. PiperOrigin-RevId: 511849585 Change-Id: I1c4acd469280628b15441a8e7a97ff572c42c1ff Co-authored-by: Googler <[email protected]>
1 parent 10e6251 commit 68e1924

File tree

7 files changed

+80
-12
lines changed

7 files changed

+80
-12
lines changed

‎src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import com.google.devtools.build.lib.events.Event;
4343
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
4444
import com.google.devtools.build.lib.events.Reporter;
45+
import com.google.devtools.build.lib.remote.common.BulkTransferException;
4546
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
4647
import com.google.devtools.build.lib.remote.util.AsyncTaskCache;
4748
import com.google.devtools.build.lib.remote.util.RxUtils.TransferResult;
@@ -252,7 +253,8 @@ private Completable prefetchInputTreeOrSymlink(
252253
PathFragment prefetchExecPath = treeMetadata.getMaterializationExecPath().orElse(execPath);
253254

254255
Completable prefetch =
255-
prefetchInputTree(context, provider, prefetchExecPath, treeFiles, treeMetadata, priority);
256+
prefetchInputTree(
257+
context, provider, prefetchExecPath, tree, treeFiles, treeMetadata, priority);
256258

257259
// If prefetching to a different path, plant a symlink into it.
258260
if (!prefetchExecPath.equals(execPath)) {
@@ -288,6 +290,7 @@ private Completable prefetchInputTree(
288290
Context context,
289291
MetadataProvider provider,
290292
PathFragment execPath,
293+
SpecialArtifact tree,
291294
List<TreeFileArtifact> treeFiles,
292295
FileArtifactValue treeMetadata,
293296
Priority priority) {
@@ -354,6 +357,12 @@ private Completable prefetchInputTree(
354357

355358
completed.set(true);
356359
})
360+
.doOnError(
361+
error -> {
362+
if (BulkTransferException.anyCausedByCacheNotFoundException(error)) {
363+
missingActionInputs.add(tree);
364+
}
365+
})
357366
.doFinally(
358367
() -> {
359368
if (!completed.get()) {

‎src/main/java/com/google/devtools/build/lib/remote/ExecuteRetrier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public int getRetryAttempts() {
8989
}
9090

9191
private static boolean shouldRetry(Exception e) {
92-
if (BulkTransferException.isOnlyCausedByCacheNotFoundException(e)) {
92+
if (BulkTransferException.allCausedByCacheNotFoundException(e)) {
9393
return true;
9494
}
9595
Status status = StatusProto.fromThrowable(e);

‎src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ protected ListenableFuture<Void> doDownloadFile(
104104
@Override
105105
protected Completable onErrorResumeNext(Throwable error) {
106106
if (error instanceof BulkTransferException) {
107-
if (((BulkTransferException) error).onlyCausedByCacheNotFoundException()) {
107+
if (((BulkTransferException) error).allCausedByCacheNotFoundException()) {
108108
var code =
109109
useNewExitCodeForLostInputs ? Code.REMOTE_CACHE_EVICTED : Code.REMOTE_CACHE_FAILED;
110110
error =

‎src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
138138
} catch (CacheNotFoundException e) {
139139
// Intentionally left blank
140140
} catch (IOException e) {
141-
if (BulkTransferException.isOnlyCausedByCacheNotFoundException(e)) {
141+
if (BulkTransferException.allCausedByCacheNotFoundException(e)) {
142142
// Intentionally left blank
143143
} else {
144144
String errorMessage;

‎src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
225225
() -> action.getNetworkTime().getDuration(),
226226
spawnMetrics);
227227
} catch (BulkTransferException e) {
228-
if (!e.onlyCausedByCacheNotFoundException()) {
228+
if (!e.allCausedByCacheNotFoundException()) {
229229
throw e;
230230
}
231231
// No cache hit, so we fall through to local or remote execution.
@@ -293,7 +293,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
293293
() -> action.getNetworkTime().getDuration(),
294294
spawnMetrics);
295295
} catch (BulkTransferException e) {
296-
if (e.onlyCausedByCacheNotFoundException()) {
296+
if (e.allCausedByCacheNotFoundException()) {
297297
// No cache hit, so if we retry this execution, we must no longer accept
298298
// cached results, it must be reexecuted
299299
useCachedResult.set(false);
@@ -517,8 +517,7 @@ private SpawnResult execLocallyAndUploadOrFail(
517517
private SpawnResult handleError(
518518
RemoteAction action, IOException exception, SpawnExecutionContext context)
519519
throws ExecException, InterruptedException, IOException {
520-
boolean remoteCacheFailed =
521-
BulkTransferException.isOnlyCausedByCacheNotFoundException(exception);
520+
boolean remoteCacheFailed = BulkTransferException.allCausedByCacheNotFoundException(exception);
522521
if (exception.getCause() instanceof ExecutionStatusException) {
523522
ExecutionStatusException e = (ExecutionStatusException) exception.getCause();
524523
if (e.getResponse() != null) {

‎src/main/java/com/google/devtools/build/lib/remote/common/BulkTransferException.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@
2323
* with all constituent exceptions available for observation.
2424
*/
2525
public class BulkTransferException extends IOException {
26-
// true since no empty BulkTransferException is ever thrown
26+
// No empty BulkTransferException is ever thrown.
2727
private boolean allCacheNotFoundException = true;
28+
private boolean anyCacheNotFoundException = false;
2829

2930
public BulkTransferException() {}
3031

@@ -40,16 +41,26 @@ public BulkTransferException(IOException e) {
4041
*/
4142
public void add(IOException e) {
4243
allCacheNotFoundException &= e instanceof CacheNotFoundException;
44+
anyCacheNotFoundException |= e instanceof CacheNotFoundException;
4345
super.addSuppressed(e);
4446
}
4547

46-
public boolean onlyCausedByCacheNotFoundException() {
48+
public boolean anyCausedByCacheNotFoundException() {
49+
return anyCacheNotFoundException;
50+
}
51+
52+
public static boolean anyCausedByCacheNotFoundException(Throwable e) {
53+
return e instanceof BulkTransferException
54+
&& ((BulkTransferException) e).anyCausedByCacheNotFoundException();
55+
}
56+
57+
public boolean allCausedByCacheNotFoundException() {
4758
return allCacheNotFoundException;
4859
}
4960

50-
public static boolean isOnlyCausedByCacheNotFoundException(Exception e) {
61+
public static boolean allCausedByCacheNotFoundException(Throwable e) {
5162
return e instanceof BulkTransferException
52-
&& ((BulkTransferException) e).onlyCausedByCacheNotFoundException();
63+
&& ((BulkTransferException) e).allCausedByCacheNotFoundException();
5364
}
5465

5566
@Override

‎src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,4 +513,53 @@ public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception
513513
// Assert: target was successfully built
514514
assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator());
515515
}
516+
517+
@Test
518+
public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() throws Exception {
519+
// Arrange: Prepare workspace and populate remote cache
520+
write("BUILD");
521+
writeOutputDirRule();
522+
write(
523+
"a/BUILD",
524+
"load('//:output_dir.bzl', 'output_dir')",
525+
"output_dir(",
526+
" name = 'foo.out',",
527+
" manifest = ':manifest',",
528+
")",
529+
"genrule(",
530+
" name = 'bar',",
531+
" srcs = ['foo.out', 'bar.in'],",
532+
" outs = ['bar.out'],",
533+
" cmd = '( ls $(location :foo.out); cat $(location :bar.in) ) > $@',",
534+
" tags = ['no-remote-exec'],",
535+
")");
536+
write("a/manifest", "file-inside");
537+
write("a/bar.in", "bar");
538+
539+
// Populate remote cache
540+
buildTarget("//a:bar");
541+
getOutputPath("a/foo.out").deleteTreesBelow();
542+
getOutputPath("a/bar.out").delete();
543+
getOutputBase().getRelative("action_cache").deleteTreesBelow();
544+
restartServer();
545+
546+
// Clean build, foo.out isn't downloaded
547+
buildTarget("//a:bar");
548+
assertOutputDoesNotExist("a/foo.out/file-inside");
549+
550+
// Evict blobs from remote cache
551+
worker.restart();
552+
553+
// trigger build error
554+
write("a/bar.in", "updated bar");
555+
// Build failed because of remote cache eviction
556+
assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar"));
557+
558+
// Act: Do an incremental build without "clean" or "shutdown"
559+
buildTarget("//a:bar");
560+
561+
// Assert: target was successfully built
562+
assertValidOutputFile(
563+
"a/bar.out", "file-inside" + lineSeparator() + "updated bar" + lineSeparator());
564+
}
516565
}

0 commit comments

Comments
 (0)