Skip to content

Commit 5406c95

Browse files
coeuvrecoeuvre
authored
[6.1.0] Cleanup stale state when remote cache evicted (bazelbuild#17538)
* 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 bazelbuild#17366. Part of bazelbuild#16660. Closes bazelbuild#17462. PiperOrigin-RevId: 510952745 Change-Id: I4fc59a21195565c68375a19ead76738d2208c4ac * Add integration tests for incremental build for Build without the Bytes. PiperOrigin-RevId: 487193345 Change-Id: Id10c9aebad630928a9a33fa22824e4d78041a4d7
1 parent 350e329 commit 5406c95

18 files changed

+478
-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
@@ -144,19 +144,11 @@ private ActionCache.Entry getCacheEntry(Action action) {
144144
if (!cacheConfig.enabled()) {
145145
return null; // ignore existing cache when disabled.
146146
}
147-
for (Artifact output : action.getOutputs()) {
148-
ActionCache.Entry entry = actionCache.get(output.getExecPathString());
149-
if (entry != null) {
150-
return entry;
151-
}
152-
}
153-
return null;
147+
return ActionCacheUtils.getCacheEntry(actionCache, action);
154148
}
155149

156150
private void removeCacheEntry(Action action) {
157-
for (Artifact output : action.getOutputs()) {
158-
actionCache.remove(output.getExecPathString());
159-
}
151+
ActionCacheUtils.removeCacheEntry(actionCache, action);
160152
}
161153

162154
@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;
@@ -60,6 +61,7 @@
6061
import java.util.Set;
6162
import java.util.concurrent.atomic.AtomicBoolean;
6263
import java.util.regex.Pattern;
64+
import javax.annotation.Nullable;
6365

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

81+
private final Set<ActionInput> missingActionInputs = Sets.newConcurrentHashSet();
82+
7983
private static class Context {
8084
private final Set<Path> nonWritableDirs = Sets.newConcurrentHashSet();
8185

@@ -387,7 +391,8 @@ private Completable prefetchInputFileOrSymlink(
387391
PathFragment prefetchExecPath = metadata.getMaterializationExecPath().orElse(execPath);
388392

389393
Completable prefetch =
390-
downloadFileNoCheckRx(context, execRoot.getRelative(prefetchExecPath), metadata, priority);
394+
downloadFileNoCheckRx(
395+
context, execRoot.getRelative(prefetchExecPath), input, metadata, priority);
391396

392397
// If prefetching to a different path, plant a symlink into it.
393398
if (!prefetchExecPath.equals(execPath)) {
@@ -406,15 +411,23 @@ private Completable prefetchInputFileOrSymlink(
406411
* download finished.
407412
*/
408413
private Completable downloadFileRx(
409-
Context context, Path path, FileArtifactValue metadata, Priority priority) {
414+
Context context,
415+
Path path,
416+
@Nullable ActionInput actionInput,
417+
FileArtifactValue metadata,
418+
Priority priority) {
410419
if (!canDownloadFile(path, metadata)) {
411420
return Completable.complete();
412421
}
413-
return downloadFileNoCheckRx(context, path, metadata, priority);
422+
return downloadFileNoCheckRx(context, path, actionInput, metadata, priority);
414423
}
415424

416425
private Completable downloadFileNoCheckRx(
417-
Context context, Path path, FileArtifactValue metadata, Priority priority) {
426+
Context context,
427+
Path path,
428+
@Nullable ActionInput actionInput,
429+
FileArtifactValue metadata,
430+
Priority priority) {
418431
if (path.isSymbolicLink()) {
419432
try {
420433
path = path.getRelative(path.readSymbolicLink());
@@ -428,26 +441,32 @@ private Completable downloadFileNoCheckRx(
428441
AtomicBoolean completed = new AtomicBoolean(false);
429442
Completable download =
430443
Completable.using(
431-
tempPathGenerator::generateTempPath,
432-
tempPath ->
433-
toCompletable(
434-
() ->
435-
doDownloadFile(
436-
tempPath, finalPath.relativeTo(execRoot), metadata, priority),
437-
directExecutor())
438-
.doOnComplete(
439-
() -> {
440-
finalizeDownload(context, tempPath, finalPath);
441-
completed.set(true);
442-
}),
443-
tempPath -> {
444-
if (!completed.get()) {
445-
deletePartialDownload(tempPath);
446-
}
447-
},
448-
// Set eager=false here because we want cleanup the download *after* upstream is
449-
// disposed.
450-
/* eager= */ false);
444+
tempPathGenerator::generateTempPath,
445+
tempPath ->
446+
toCompletable(
447+
() ->
448+
doDownloadFile(
449+
tempPath, finalPath.relativeTo(execRoot), metadata, priority),
450+
directExecutor())
451+
.doOnComplete(
452+
() -> {
453+
finalizeDownload(context, tempPath, finalPath);
454+
completed.set(true);
455+
}),
456+
tempPath -> {
457+
if (!completed.get()) {
458+
deletePartialDownload(tempPath);
459+
}
460+
},
461+
// Set eager=false here because we want cleanup the download *after* upstream is
462+
// disposed.
463+
/* eager= */ false)
464+
.doOnError(
465+
error -> {
466+
if (error instanceof CacheNotFoundException && actionInput != null) {
467+
missingActionInputs.add(actionInput);
468+
}
469+
});
451470

452471
return downloadCache.executeIfNot(
453472
finalPath,
@@ -467,19 +486,27 @@ private Completable downloadFileNoCheckRx(
467486
* <p>The file will be written into a temporary file and moved to the final destination after the
468487
* download finished.
469488
*/
470-
public void downloadFile(Path path, FileArtifactValue metadata)
489+
public void downloadFile(Path path, @Nullable ActionInput actionInput, FileArtifactValue metadata)
471490
throws IOException, InterruptedException {
472-
getFromFuture(downloadFileAsync(path.asFragment(), metadata, Priority.CRITICAL));
491+
getFromFuture(downloadFileAsync(path.asFragment(), actionInput, metadata, Priority.CRITICAL));
473492
}
474493

475494
protected ListenableFuture<Void> downloadFileAsync(
476-
PathFragment path, FileArtifactValue metadata, Priority priority) {
495+
PathFragment path,
496+
@Nullable ActionInput actionInput,
497+
FileArtifactValue metadata,
498+
Priority priority) {
477499
Context context = new Context();
478500
return toListenableFuture(
479501
Completable.using(
480502
() -> context,
481503
ctx ->
482-
downloadFileRx(context, execRoot.getFileSystem().getPath(path), metadata, priority),
504+
downloadFileRx(
505+
context,
506+
execRoot.getFileSystem().getPath(path),
507+
actionInput,
508+
metadata,
509+
priority),
483510
Context::finalizeContext));
484511
}
485512

@@ -636,4 +663,8 @@ private boolean outputMatchesPattern(Artifact output) {
636663
public void flushOutputTree() throws InterruptedException {
637664
downloadCache.awaitInProgressTasks();
638665
}
666+
667+
public ImmutableSet<ActionInput> getMissingActionInputs() {
668+
return ImmutableSet.copyOf(missingActionInputs);
669+
}
639670
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ java_library(
5252
"//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory",
5353
"//src/main/java/com/google/devtools/build/lib/actions",
5454
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
55+
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
5556
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
5657
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
5758
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
@@ -183,11 +184,13 @@ java_library(
183184
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
184185
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
185186
"//src/main/java/com/google/devtools/build/lib/events",
187+
"//src/main/java/com/google/devtools/build/lib/remote/common",
186188
"//src/main/java/com/google/devtools/build/lib/remote/util",
187189
"//src/main/java/com/google/devtools/build/lib/vfs",
188190
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
189191
"//third_party:flogger",
190192
"//third_party:guava",
193+
"//third_party:jsr305",
191194
"//third_party:rxjava3",
192195
],
193196
)

‎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;
@@ -538,6 +539,12 @@ public RemoteFileArtifactValue getRemoteMetadata() {
538539
};
539540
}
540541

542+
@Nullable
543+
protected ActionInput getActionInput(PathFragment path) {
544+
PathFragment execPath = path.relativeTo(execRoot);
545+
return inputArtifactData.getInput(execPath.getPathString());
546+
}
547+
541548
@Nullable
542549
protected RemoteFileArtifactValue getRemoteMetadata(PathFragment path) {
543550
if (!isOutput(path)) {
@@ -577,7 +584,7 @@ private void downloadFileIfRemote(PathFragment path) throws IOException {
577584
FileArtifactValue m = getRemoteMetadata(path);
578585
if (m != null) {
579586
try {
580-
inputFetcher.downloadFile(delegateFs.getPath(path), m);
587+
inputFetcher.downloadFile(delegateFs.getPath(path), getActionInput(path), m);
581588
} catch (InterruptedException e) {
582589
Thread.currentThread().interrupt();
583590
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;
@@ -58,6 +60,7 @@
5860
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
5961
import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
6062
import com.google.devtools.build.lib.remote.RemoteServerCapabilities.ServerCapabilitiesRequirement;
63+
import com.google.devtools.build.lib.remote.ToplevelArtifactsDownloader.PathToMetadataConverter;
6164
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
6265
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
6366
import com.google.devtools.build.lib.remote.downloader.GrpcRemoteDownloader;
@@ -921,7 +924,6 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
921924
remoteOptions.useNewExitCodeForLostInputs);
922925
env.getEventBus().register(actionInputFetcher);
923926
builder.setActionInputPrefetcher(actionInputFetcher);
924-
remoteOutputService.setActionInputFetcher(actionInputFetcher);
925927
actionContextProvider.setActionInputFetcher(actionInputFetcher);
926928

927929
toplevelArtifactsDownloader =
@@ -930,15 +932,35 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
930932
remoteOutputsMode.downloadToplevelOutputsOnly(),
931933
env.getSkyframeExecutor().getEvaluator(),
932934
actionInputFetcher,
933-
(path) -> {
934-
FileSystem fileSystem = path.getFileSystem();
935-
if (fileSystem instanceof RemoteActionFileSystem) {
936-
return ((RemoteActionFileSystem) path.getFileSystem())
937-
.getRemoteMetadata(path.asFragment());
935+
new PathToMetadataConverter() {
936+
@Nullable
937+
@Override
938+
public FileArtifactValue getMetadata(Path path) {
939+
FileSystem fileSystem = path.getFileSystem();
940+
if (fileSystem instanceof RemoteActionFileSystem) {
941+
return ((RemoteActionFileSystem) path.getFileSystem())
942+
.getRemoteMetadata(path.asFragment());
943+
}
944+
return null;
945+
}
946+
947+
@Nullable
948+
@Override
949+
public ActionInput getActionInput(Path path) {
950+
FileSystem fileSystem = path.getFileSystem();
951+
if (fileSystem instanceof RemoteActionFileSystem) {
952+
return ((RemoteActionFileSystem) path.getFileSystem())
953+
.getActionInput(path.asFragment());
954+
}
955+
return null;
938956
}
939-
return null;
940957
});
941958
env.getEventBus().register(toplevelArtifactsDownloader);
959+
960+
remoteOutputService.setActionInputFetcher(actionInputFetcher);
961+
remoteOutputService.setMemoizingEvaluator(env.getSkyframeExecutor().getEvaluator());
962+
remoteOutputService.setActionCache(env.getBlazeWorkspace().getPersistentActionCache());
963+
env.getEventBus().register(remoteOutputService);
942964
}
943965
}
944966

0 commit comments

Comments
 (0)