Skip to content

Commit 963640a

Browse files
coeuvrecopybara-github
coeuvre
authored and
copybara-github
committed
Cleanup stale state when remote cache evicted
Currently, when building without the bytes, if Bazel failed to download blobs from CAS when fetching them as inputs to local actions, Bazel fails the build with message like `... --remote_download_outputs=minimal does not work if your remote cache evicts files during builds.` and this message keep showing up until a manually `bazel clean`. This PR fixes that by cleaning up stale state in skyframe and action cache upon remote cache eviction so that a following build can continue without `bazel shutdown` or `bazel clean`. Fixes #17366. Part of #16660. Closes #17462. PiperOrigin-RevId: 510952745 Change-Id: I4fc59a21195565c68375a19ead76738d2208c4ac
1 parent 669c298 commit 963640a

File tree

17 files changed

+353
-77
lines changed

17 files changed

+353
-77
lines changed

‎src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -145,19 +145,11 @@ private ActionCache.Entry getCacheEntry(Action action) {
145145
if (!cacheConfig.enabled()) {
146146
return null; // ignore existing cache when disabled.
147147
}
148-
for (Artifact output : action.getOutputs()) {
149-
ActionCache.Entry entry = actionCache.get(output.getExecPathString());
150-
if (entry != null) {
151-
return entry;
152-
}
153-
}
154-
return null;
148+
return ActionCacheUtils.getCacheEntry(actionCache, action);
155149
}
156150

157151
private void removeCacheEntry(Action action) {
158-
for (Artifact output : action.getOutputs()) {
159-
actionCache.remove(output.getExecPathString());
160-
}
152+
ActionCacheUtils.removeCacheEntry(actionCache, action);
161153
}
162154

163155
@Nullable

‎src/main/java/com/google/devtools/build/lib/actions/ActionCacheUtils.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright 2023 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.actions;
15+
16+
import com.google.devtools.build.lib.actions.cache.ActionCache;
17+
import javax.annotation.Nullable;
18+
19+
/** Utility functions for {@link ActionCache}. */
20+
public class ActionCacheUtils {
21+
private ActionCacheUtils() {}
22+
23+
/** Checks whether one of existing output paths is already used as a key. */
24+
@Nullable
25+
public static ActionCache.Entry getCacheEntry(ActionCache actionCache, Action action) {
26+
for (Artifact output : action.getOutputs()) {
27+
ActionCache.Entry entry = actionCache.get(output.getExecPathString());
28+
if (entry != null) {
29+
return entry;
30+
}
31+
}
32+
return null;
33+
}
34+
35+
public static void removeCacheEntry(ActionCache actionCache, Action action) {
36+
for (Artifact output : action.getOutputs()) {
37+
actionCache.remove(output.getExecPathString());
38+
}
39+
}
40+
}

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

Lines changed: 59 additions & 28 deletions
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.CacheNotFoundException;
4546
import com.google.devtools.build.lib.remote.util.AsyncTaskCache;
4647
import com.google.devtools.build.lib.remote.util.RxUtils.TransferResult;
4748
import com.google.devtools.build.lib.remote.util.TempPathGenerator;
@@ -61,6 +62,7 @@
6162
import java.util.Set;
6263
import java.util.concurrent.atomic.AtomicBoolean;
6364
import java.util.regex.Pattern;
65+
import javax.annotation.Nullable;
6466

6567
/**
6668
* Abstract implementation of {@link ActionInputPrefetcher} which implements the orchestration of
@@ -78,6 +80,8 @@ public abstract class AbstractActionInputPrefetcher implements ActionInputPrefet
7880
protected final Path execRoot;
7981
protected final ImmutableList<Pattern> patternsToDownload;
8082

83+
private final Set<ActionInput> missingActionInputs = Sets.newConcurrentHashSet();
84+
8185
private static class Context {
8286
private final Set<Path> nonWritableDirs = Sets.newConcurrentHashSet();
8387

@@ -399,7 +403,8 @@ private Completable prefetchInputFileOrSymlink(
399403
PathFragment prefetchExecPath = metadata.getMaterializationExecPath().orElse(execPath);
400404

401405
Completable prefetch =
402-
downloadFileNoCheckRx(context, execRoot.getRelative(prefetchExecPath), metadata, priority);
406+
downloadFileNoCheckRx(
407+
context, execRoot.getRelative(prefetchExecPath), input, metadata, priority);
403408

404409
// If prefetching to a different path, plant a symlink into it.
405410
if (!prefetchExecPath.equals(execPath)) {
@@ -418,15 +423,23 @@ private Completable prefetchInputFileOrSymlink(
418423
* download finished.
419424
*/
420425
private Completable downloadFileRx(
421-
Context context, Path path, FileArtifactValue metadata, Priority priority) {
426+
Context context,
427+
Path path,
428+
@Nullable ActionInput actionInput,
429+
FileArtifactValue metadata,
430+
Priority priority) {
422431
if (!canDownloadFile(path, metadata)) {
423432
return Completable.complete();
424433
}
425-
return downloadFileNoCheckRx(context, path, metadata, priority);
434+
return downloadFileNoCheckRx(context, path, actionInput, metadata, priority);
426435
}
427436

428437
private Completable downloadFileNoCheckRx(
429-
Context context, Path path, FileArtifactValue metadata, Priority priority) {
438+
Context context,
439+
Path path,
440+
@Nullable ActionInput actionInput,
441+
FileArtifactValue metadata,
442+
Priority priority) {
430443
if (path.isSymbolicLink()) {
431444
try {
432445
path = path.getRelative(path.readSymbolicLink());
@@ -440,26 +453,32 @@ private Completable downloadFileNoCheckRx(
440453
AtomicBoolean completed = new AtomicBoolean(false);
441454
Completable download =
442455
Completable.using(
443-
tempPathGenerator::generateTempPath,
444-
tempPath ->
445-
toCompletable(
446-
() ->
447-
doDownloadFile(
448-
tempPath, finalPath.relativeTo(execRoot), metadata, priority),
449-
directExecutor())
450-
.doOnComplete(
451-
() -> {
452-
finalizeDownload(context, tempPath, finalPath);
453-
completed.set(true);
454-
}),
455-
tempPath -> {
456-
if (!completed.get()) {
457-
deletePartialDownload(tempPath);
458-
}
459-
},
460-
// Set eager=false here because we want cleanup the download *after* upstream is
461-
// disposed.
462-
/* eager= */ false);
456+
tempPathGenerator::generateTempPath,
457+
tempPath ->
458+
toCompletable(
459+
() ->
460+
doDownloadFile(
461+
tempPath, finalPath.relativeTo(execRoot), metadata, priority),
462+
directExecutor())
463+
.doOnComplete(
464+
() -> {
465+
finalizeDownload(context, tempPath, finalPath);
466+
completed.set(true);
467+
}),
468+
tempPath -> {
469+
if (!completed.get()) {
470+
deletePartialDownload(tempPath);
471+
}
472+
},
473+
// Set eager=false here because we want cleanup the download *after* upstream is
474+
// disposed.
475+
/* eager= */ false)
476+
.doOnError(
477+
error -> {
478+
if (error instanceof CacheNotFoundException && actionInput != null) {
479+
missingActionInputs.add(actionInput);
480+
}
481+
});
463482

464483
return downloadCache.executeIfNot(
465484
finalPath,
@@ -479,19 +498,27 @@ private Completable downloadFileNoCheckRx(
479498
* <p>The file will be written into a temporary file and moved to the final destination after the
480499
* download finished.
481500
*/
482-
public void downloadFile(Path path, FileArtifactValue metadata)
501+
public void downloadFile(Path path, @Nullable ActionInput actionInput, FileArtifactValue metadata)
483502
throws IOException, InterruptedException {
484-
getFromFuture(downloadFileAsync(path.asFragment(), metadata, Priority.CRITICAL));
503+
getFromFuture(downloadFileAsync(path.asFragment(), actionInput, metadata, Priority.CRITICAL));
485504
}
486505

487506
protected ListenableFuture<Void> downloadFileAsync(
488-
PathFragment path, FileArtifactValue metadata, Priority priority) {
507+
PathFragment path,
508+
@Nullable ActionInput actionInput,
509+
FileArtifactValue metadata,
510+
Priority priority) {
489511
Context context = new Context();
490512
return toListenableFuture(
491513
Completable.using(
492514
() -> context,
493515
ctx ->
494-
downloadFileRx(context, execRoot.getFileSystem().getPath(path), metadata, priority),
516+
downloadFileRx(
517+
context,
518+
execRoot.getFileSystem().getPath(path),
519+
actionInput,
520+
metadata,
521+
priority),
495522
Context::finalizeContext));
496523
}
497524

@@ -648,4 +675,8 @@ private boolean outputMatchesPattern(Artifact output) {
648675
public void flushOutputTree() throws InterruptedException {
649676
downloadCache.awaitInProgressTasks();
650677
}
678+
679+
public ImmutableSet<ActionInput> getMissingActionInputs() {
680+
return ImmutableSet.copyOf(missingActionInputs);
681+
}
651682
}

‎src/main/java/com/google/devtools/build/lib/remote/BUILD‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ java_library(
5353
"//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory",
5454
"//src/main/java/com/google/devtools/build/lib/actions",
5555
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
56+
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
5657
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
5758
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
5859
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
@@ -185,11 +186,13 @@ java_library(
185186
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
186187
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
187188
"//src/main/java/com/google/devtools/build/lib/events",
189+
"//src/main/java/com/google/devtools/build/lib/remote/common",
188190
"//src/main/java/com/google/devtools/build/lib/remote/util",
189191
"//src/main/java/com/google/devtools/build/lib/vfs",
190192
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
191193
"//third_party:flogger",
192194
"//third_party:guava",
195+
"//third_party:jsr305",
193196
"//third_party:rxjava3",
194197
],
195198
)

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.common.annotations.VisibleForTesting;
2525
import com.google.common.collect.ImmutableList;
2626
import com.google.common.collect.ImmutableMap;
27+
import com.google.devtools.build.lib.actions.ActionInput;
2728
import com.google.devtools.build.lib.actions.ActionInputMap;
2829
import com.google.devtools.build.lib.actions.Artifact;
2930
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
@@ -533,6 +534,12 @@ public RemoteFileArtifactValue getRemoteMetadata() {
533534
};
534535
}
535536

537+
@Nullable
538+
protected ActionInput getActionInput(PathFragment path) {
539+
PathFragment execPath = path.relativeTo(execRoot);
540+
return inputArtifactData.getInput(execPath.getPathString());
541+
}
542+
536543
@Nullable
537544
protected RemoteFileArtifactValue getRemoteMetadata(PathFragment path) {
538545
if (!isOutput(path)) {
@@ -572,7 +579,7 @@ private void downloadFileIfRemote(PathFragment path) throws IOException {
572579
FileArtifactValue m = getRemoteMetadata(path);
573580
if (m != null) {
574581
try {
575-
inputFetcher.downloadFile(delegateFs.getPath(path), m);
582+
inputFetcher.downloadFile(delegateFs.getPath(path), getActionInput(path), m);
576583
} catch (InterruptedException e) {
577584
Thread.currentThread().interrupt();
578585
throw new IOException(

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,21 @@ public ListenableFuture<Void> uploadActionResult(
151151
*/
152152
public ListenableFuture<Void> uploadFile(
153153
RemoteActionExecutionContext context, Digest digest, Path file) {
154+
return uploadFile(context, digest, file, /* force= */ false);
155+
}
156+
157+
protected ListenableFuture<Void> uploadFile(
158+
RemoteActionExecutionContext context, Digest digest, Path file, boolean force) {
154159
if (digest.getSizeBytes() == 0) {
155160
return COMPLETED_SUCCESS;
156161
}
157162

158163
Completable upload =
159-
casUploadCache.executeIfNot(
164+
casUploadCache.execute(
160165
digest,
161166
RxFutures.toCompletable(
162-
() -> cacheProtocol.uploadFile(context, digest, file), directExecutor()));
167+
() -> cacheProtocol.uploadFile(context, digest, file), directExecutor()),
168+
force);
163169

164170
return RxFutures.toListenableFuture(upload);
165171
}
@@ -176,15 +182,21 @@ public ListenableFuture<Void> uploadFile(
176182
*/
177183
public ListenableFuture<Void> uploadBlob(
178184
RemoteActionExecutionContext context, Digest digest, ByteString data) {
185+
return uploadBlob(context, digest, data, /* force= */ false);
186+
}
187+
188+
protected ListenableFuture<Void> uploadBlob(
189+
RemoteActionExecutionContext context, Digest digest, ByteString data, boolean force) {
179190
if (digest.getSizeBytes() == 0) {
180191
return COMPLETED_SUCCESS;
181192
}
182193

183194
Completable upload =
184-
casUploadCache.executeIfNot(
195+
casUploadCache.execute(
185196
digest,
186197
RxFutures.toCompletable(
187-
() -> cacheProtocol.uploadBlob(context, digest, data), directExecutor()));
198+
() -> cacheProtocol.uploadBlob(context, digest, data), directExecutor()),
199+
force);
188200

189201
return RxFutures.toListenableFuture(upload);
190202
}

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

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
import com.google.common.util.concurrent.MoreExecutors;
3535
import com.google.common.util.concurrent.ThreadFactoryBuilder;
3636
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
37+
import com.google.devtools.build.lib.actions.ActionInput;
3738
import com.google.devtools.build.lib.actions.Artifact;
39+
import com.google.devtools.build.lib.actions.FileArtifactValue;
3840
import com.google.devtools.build.lib.analysis.AnalysisResult;
3941
import com.google.devtools.build.lib.analysis.BlazeDirectories;
4042
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -59,6 +61,7 @@
5961
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
6062
import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
6163
import com.google.devtools.build.lib.remote.RemoteServerCapabilities.ServerCapabilitiesRequirement;
64+
import com.google.devtools.build.lib.remote.ToplevelArtifactsDownloader.PathToMetadataConverter;
6265
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
6366
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
6467
import com.google.devtools.build.lib.remote.downloader.GrpcRemoteDownloader;
@@ -987,7 +990,6 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
987990
remoteOptions.useNewExitCodeForLostInputs);
988991
env.getEventBus().register(actionInputFetcher);
989992
builder.setActionInputPrefetcher(actionInputFetcher);
990-
remoteOutputService.setActionInputFetcher(actionInputFetcher);
991993
actionContextProvider.setActionInputFetcher(actionInputFetcher);
992994

993995
toplevelArtifactsDownloader =
@@ -996,15 +998,35 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
996998
remoteOutputsMode.downloadToplevelOutputsOnly(),
997999
env.getSkyframeExecutor().getEvaluator(),
9981000
actionInputFetcher,
999-
(path) -> {
1000-
FileSystem fileSystem = path.getFileSystem();
1001-
if (fileSystem instanceof RemoteActionFileSystem) {
1002-
return ((RemoteActionFileSystem) path.getFileSystem())
1003-
.getRemoteMetadata(path.asFragment());
1001+
new PathToMetadataConverter() {
1002+
@Nullable
1003+
@Override
1004+
public FileArtifactValue getMetadata(Path path) {
1005+
FileSystem fileSystem = path.getFileSystem();
1006+
if (fileSystem instanceof RemoteActionFileSystem) {
1007+
return ((RemoteActionFileSystem) path.getFileSystem())
1008+
.getRemoteMetadata(path.asFragment());
1009+
}
1010+
return null;
1011+
}
1012+
1013+
@Nullable
1014+
@Override
1015+
public ActionInput getActionInput(Path path) {
1016+
FileSystem fileSystem = path.getFileSystem();
1017+
if (fileSystem instanceof RemoteActionFileSystem) {
1018+
return ((RemoteActionFileSystem) path.getFileSystem())
1019+
.getActionInput(path.asFragment());
1020+
}
1021+
return null;
10041022
}
1005-
return null;
10061023
});
10071024
env.getEventBus().register(toplevelArtifactsDownloader);
1025+
1026+
remoteOutputService.setActionInputFetcher(actionInputFetcher);
1027+
remoteOutputService.setMemoizingEvaluator(env.getSkyframeExecutor().getEvaluator());
1028+
remoteOutputService.setActionCache(env.getBlazeWorkspace().getPersistentActionCache());
1029+
env.getEventBus().register(remoteOutputService);
10081030
}
10091031
}
10101032

0 commit comments

Comments
 (0)