Skip to content

Commit aafe123

Browse files
coeuvreShreeM01
andauthored
[6.1.0] Handle remote cache eviction when uploading inputs for remote actions. (bazelbuild#17605)
* Extract code for cleaning stale state when remote CAS evicted blobs into another class. PiperOrigin-RevId: 512049304 Change-Id: I87e9e6bcd4d4c4d8be27be13cfb8ba70b199b6e6 * Handle remote cache eviction when uploading inputs for remote actions. Previously, we handle remote cache eviction when downloading inputs for local actions. However, it's possible that when Bazel need to re-execute an remote action, it detected that some inputs are missing from remote CAS. In this case, Bazel will try to upload inputs by reading from local filesystem. Since the inputs were generated remotely, not downloaded and evicted remotely, the upload will fail with FileNotFoundException. This CL changes the code to correctly handles above case by reading through ActionFS when uploading inputs and propagate CacheNotFoundException. Related to bazelbuild#16660. PiperOrigin-RevId: 512568547 Change-Id: I3a28cadbb6285fa3727e1603f37abf8843c093c9 * Fix tests --------- Co-authored-by: kshyanashree <[email protected]>
1 parent 7e328bb commit aafe123

28 files changed

+452
-150
lines changed

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

+16-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ java_library(
3434
"Retrier.java",
3535
"AbstractActionInputPrefetcher.java",
3636
"ToplevelArtifactsDownloader.java",
37+
"LeaseService.java",
3738
],
3839
),
3940
exports = [
@@ -46,13 +47,13 @@ java_library(
4647
":ReferenceCountedChannel",
4748
":Retrier",
4849
":abstract_action_input_prefetcher",
50+
":lease_service",
4951
":toplevel_artifacts_downloader",
5052
"//src/main/java/com/google/devtools/build/lib:build-request-options",
5153
"//src/main/java/com/google/devtools/build/lib:runtime",
5254
"//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory",
5355
"//src/main/java/com/google/devtools/build/lib/actions",
5456
"//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",
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",
@@ -83,6 +84,7 @@ java_library(
8384
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
8485
"//src/main/java/com/google/devtools/build/lib/profiler",
8586
"//src/main/java/com/google/devtools/build/lib/remote/common",
87+
"//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception",
8688
"//src/main/java/com/google/devtools/build/lib/remote/disk",
8789
"//src/main/java/com/google/devtools/build/lib/remote/downloader",
8890
"//src/main/java/com/google/devtools/build/lib/remote/grpc",
@@ -185,6 +187,7 @@ java_library(
185187
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
186188
"//src/main/java/com/google/devtools/build/lib/events",
187189
"//src/main/java/com/google/devtools/build/lib/remote/common",
190+
"//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception",
188191
"//src/main/java/com/google/devtools/build/lib/remote/util",
189192
"//src/main/java/com/google/devtools/build/lib/vfs",
190193
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
@@ -220,3 +223,15 @@ java_library(
220223
"//third_party:jsr305",
221224
],
222225
)
226+
227+
java_library(
228+
name = "lease_service",
229+
srcs = ["LeaseService.java"],
230+
deps = [
231+
"//src/main/java/com/google/devtools/build/lib/actions",
232+
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
233+
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
234+
"//src/main/java/com/google/devtools/build/skyframe",
235+
"//third_party:jsr305",
236+
],
237+
)

src/main/java/com/google/devtools/build/lib/remote/Chunker.java

+13-31
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import java.io.PushbackInputStream;
3535
import java.util.NoSuchElementException;
3636
import java.util.Objects;
37-
import java.util.function.Supplier;
3837

3938
/**
4039
* Splits a data source into one or more {@link Chunk}s of at most {@code chunkSize} bytes.
@@ -92,8 +91,7 @@ public boolean equals(Object o) {
9291
return false;
9392
}
9493
Chunk other = (Chunk) o;
95-
return other.offset == offset
96-
&& other.data.equals(data);
94+
return other.offset == offset && other.data.equals(data);
9795
}
9896

9997
@Override
@@ -102,7 +100,12 @@ public int hashCode() {
102100
}
103101
}
104102

105-
private final Supplier<InputStream> dataSupplier;
103+
/** A supplier that provide data as {@link InputStream}. */
104+
public interface ChunkDataSupplier {
105+
InputStream get() throws IOException;
106+
}
107+
108+
private final ChunkDataSupplier dataSupplier;
106109
private final long size;
107110
private final int chunkSize;
108111
private final Chunk emptyChunk;
@@ -117,7 +120,7 @@ public int hashCode() {
117120
// lazily on the first call to next(), as opposed to opening it in the constructor or on reset().
118121
private boolean initialized;
119122

120-
Chunker(Supplier<InputStream> dataSupplier, long size, int chunkSize, boolean compressed) {
123+
Chunker(ChunkDataSupplier dataSupplier, long size, int chunkSize, boolean compressed) {
121124
this.dataSupplier = checkNotNull(dataSupplier);
122125
this.size = size;
123126
this.chunkSize = chunkSize;
@@ -287,7 +290,7 @@ public static class Builder {
287290
private int chunkSize = getDefaultChunkSize();
288291
protected long size;
289292
private boolean compressed;
290-
protected Supplier<InputStream> inputStream;
293+
protected ChunkDataSupplier inputStream;
291294

292295
@CanIgnoreReturnValue
293296
public Builder setInput(byte[] data) {
@@ -310,14 +313,7 @@ public Builder setInput(long size, InputStream in) {
310313
public Builder setInput(long size, Path file) {
311314
checkState(inputStream == null);
312315
this.size = size;
313-
inputStream =
314-
() -> {
315-
try {
316-
return file.getInputStream();
317-
} catch (IOException e) {
318-
throw new RuntimeException(e);
319-
}
320-
};
316+
inputStream = file::getInputStream;
321317
return this;
322318
}
323319

@@ -326,30 +322,16 @@ public Builder setInput(long size, ActionInput actionInput, Path execRoot) {
326322
checkState(inputStream == null);
327323
this.size = size;
328324
if (actionInput instanceof VirtualActionInput) {
329-
inputStream =
330-
() -> {
331-
try {
332-
return ((VirtualActionInput) actionInput).getBytes().newInput();
333-
} catch (IOException e) {
334-
throw new RuntimeException(e);
335-
}
336-
};
325+
inputStream = () -> ((VirtualActionInput) actionInput).getBytes().newInput();
337326
} else {
338-
inputStream =
339-
() -> {
340-
try {
341-
return ActionInputHelper.toInputPath(actionInput, execRoot).getInputStream();
342-
} catch (IOException e) {
343-
throw new RuntimeException(e);
344-
}
345-
};
327+
inputStream = () -> ActionInputHelper.toInputPath(actionInput, execRoot).getInputStream();
346328
}
347329
return this;
348330
}
349331

350332
@CanIgnoreReturnValue
351333
@VisibleForTesting
352-
protected final Builder setInputSupplier(Supplier<InputStream> inputStream) {
334+
protected final Builder setInputSupplier(ChunkDataSupplier inputStream) {
353335
this.inputStream = inputStream;
354336
return this;
355337
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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.remote;
15+
16+
import com.google.devtools.build.lib.actions.Action;
17+
import com.google.devtools.build.lib.actions.ActionCacheUtils;
18+
import com.google.devtools.build.lib.actions.ActionInput;
19+
import com.google.devtools.build.lib.actions.ActionLookupData;
20+
import com.google.devtools.build.lib.actions.ActionLookupValue;
21+
import com.google.devtools.build.lib.actions.Artifact;
22+
import com.google.devtools.build.lib.actions.cache.ActionCache;
23+
import com.google.devtools.build.skyframe.MemoizingEvaluator;
24+
import java.util.HashMap;
25+
import java.util.Set;
26+
import javax.annotation.Nullable;
27+
28+
/** A lease service that manages the lease of remote blobs. */
29+
public class LeaseService {
30+
private final MemoizingEvaluator memoizingEvaluator;
31+
@Nullable private final ActionCache actionCache;
32+
33+
public LeaseService(MemoizingEvaluator memoizingEvaluator, @Nullable ActionCache actionCache) {
34+
this.memoizingEvaluator = memoizingEvaluator;
35+
this.actionCache = actionCache;
36+
}
37+
38+
/** Clean up internal state when files are evicted from remote CAS. */
39+
public void handleMissingInputs(Set<ActionInput> missingActionInputs) {
40+
if (missingActionInputs.isEmpty()) {
41+
return;
42+
}
43+
44+
var actions = new HashMap<ActionLookupData, Action>();
45+
46+
try {
47+
for (ActionInput actionInput : missingActionInputs) {
48+
if (actionInput instanceof Artifact.DerivedArtifact) {
49+
Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) actionInput;
50+
ActionLookupData actionLookupData = output.getGeneratingActionKey();
51+
var actionLookupValue =
52+
memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey());
53+
if (actionLookupValue instanceof ActionLookupValue) {
54+
Action action =
55+
((ActionLookupValue) actionLookupValue)
56+
.getAction(actionLookupData.getActionIndex());
57+
actions.put(actionLookupData, action);
58+
}
59+
}
60+
}
61+
} catch (InterruptedException e) {
62+
Thread.currentThread().interrupt();
63+
}
64+
65+
if (!actions.isEmpty()) {
66+
var actionKeys = actions.keySet();
67+
memoizingEvaluator.delete(key -> key instanceof ActionLookupData && actionKeys.contains(key));
68+
69+
if (actionCache != null) {
70+
for (var action : actions.values()) {
71+
ActionCacheUtils.removeCacheEntry(actionCache, action);
72+
}
73+
}
74+
}
75+
}
76+
}

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

+25-7
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import com.google.common.util.concurrent.ListenableFuture;
6565
import com.google.devtools.build.lib.actions.ActionInput;
6666
import com.google.devtools.build.lib.actions.Artifact;
67+
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
6768
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
6869
import com.google.devtools.build.lib.actions.ExecException;
6970
import com.google.devtools.build.lib.actions.ExecutionRequirements;
@@ -371,10 +372,14 @@ private MerkleTree buildInputMerkleTree(
371372
spawn,
372373
context,
373374
(Object nodeKey, InputWalker walker) -> {
374-
subMerkleTrees.add(buildMerkleTreeVisitor(nodeKey, walker, metadataProvider));
375+
subMerkleTrees.add(
376+
buildMerkleTreeVisitor(
377+
nodeKey, walker, metadataProvider, context.getPathResolver()));
375378
});
376379
if (!outputDirMap.isEmpty()) {
377-
subMerkleTrees.add(MerkleTree.build(outputDirMap, metadataProvider, execRoot, digestUtil));
380+
subMerkleTrees.add(
381+
MerkleTree.build(
382+
outputDirMap, metadataProvider, execRoot, context.getPathResolver(), digestUtil));
378383
}
379384
return MerkleTree.merge(subMerkleTrees, digestUtil);
380385
} else {
@@ -394,31 +399,44 @@ private MerkleTree buildInputMerkleTree(
394399
toolSignature == null ? ImmutableSet.of() : toolSignature.toolInputs,
395400
context.getMetadataProvider(),
396401
execRoot,
402+
context.getPathResolver(),
397403
digestUtil);
398404
}
399405
}
400406

401407
private MerkleTree buildMerkleTreeVisitor(
402-
Object nodeKey, InputWalker walker, MetadataProvider metadataProvider)
408+
Object nodeKey,
409+
InputWalker walker,
410+
MetadataProvider metadataProvider,
411+
ArtifactPathResolver artifactPathResolver)
403412
throws IOException, ForbiddenActionInputException {
404413
MerkleTree result = merkleTreeCache.getIfPresent(nodeKey);
405414
if (result == null) {
406-
result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider);
415+
result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider, artifactPathResolver);
407416
merkleTreeCache.put(nodeKey, result);
408417
}
409418
return result;
410419
}
411420

412421
@VisibleForTesting
413422
public MerkleTree uncachedBuildMerkleTreeVisitor(
414-
InputWalker walker, MetadataProvider metadataProvider)
423+
InputWalker walker,
424+
MetadataProvider metadataProvider,
425+
ArtifactPathResolver artifactPathResolver)
415426
throws IOException, ForbiddenActionInputException {
416427
ConcurrentLinkedQueue<MerkleTree> subMerkleTrees = new ConcurrentLinkedQueue<>();
417428
subMerkleTrees.add(
418-
MerkleTree.build(walker.getLeavesInputMapping(), metadataProvider, execRoot, digestUtil));
429+
MerkleTree.build(
430+
walker.getLeavesInputMapping(),
431+
metadataProvider,
432+
execRoot,
433+
artifactPathResolver,
434+
digestUtil));
419435
walker.visitNonLeaves(
420436
(Object subNodeKey, InputWalker subWalker) -> {
421-
subMerkleTrees.add(buildMerkleTreeVisitor(subNodeKey, subWalker, metadataProvider));
437+
subMerkleTrees.add(
438+
buildMerkleTreeVisitor(
439+
subNodeKey, subWalker, metadataProvider, artifactPathResolver));
422440
});
423441
return MerkleTree.merge(subMerkleTrees, digestUtil);
424442
}

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -957,9 +957,13 @@ public ActionInput getActionInput(Path path) {
957957
});
958958
env.getEventBus().register(toplevelArtifactsDownloader);
959959

960+
var leaseService =
961+
new LeaseService(
962+
env.getSkyframeExecutor().getEvaluator(),
963+
env.getBlazeWorkspace().getPersistentActionCache());
964+
960965
remoteOutputService.setActionInputFetcher(actionInputFetcher);
961-
remoteOutputService.setMemoizingEvaluator(env.getSkyframeExecutor().getEvaluator());
962-
remoteOutputService.setActionCache(env.getBlazeWorkspace().getPersistentActionCache());
966+
remoteOutputService.setLeaseService(leaseService);
963967
env.getEventBus().register(remoteOutputService);
964968
}
965969
}

0 commit comments

Comments
 (0)