From aafe1235c55f6cdcfc577a40736aaeb9ebaca23b Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Tue, 28 Feb 2023 18:00:03 +0100 Subject: [PATCH] [6.1.0] Handle remote cache eviction when uploading inputs for remote actions. (#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 #16660. PiperOrigin-RevId: 512568547 Change-Id: I3a28cadbb6285fa3727e1603f37abf8843c093c9 * Fix tests --------- Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com> --- .../google/devtools/build/lib/remote/BUILD | 17 ++- .../devtools/build/lib/remote/Chunker.java | 44 ++----- .../build/lib/remote/LeaseService.java | 76 ++++++++++++ .../lib/remote/RemoteExecutionService.java | 32 +++-- .../build/lib/remote/RemoteModule.java | 8 +- .../build/lib/remote/RemoteOutputService.java | 58 +-------- .../build/lib/remote/RemoteSpawnRunner.java | 6 +- .../devtools/build/lib/remote/common/BUILD | 15 ++- .../devtools/build/lib/remote/disk/BUILD | 2 +- .../devtools/build/lib/remote/http/BUILD | 1 + .../merkletree/DirectoryTreeBuilder.java | 22 +++- .../lib/remote/merkletree/MerkleTree.java | 8 +- .../devtools/build/lib/remote/util/BUILD | 1 + .../remote/ActionInputPrefetcherTestBase.java | 16 ++- .../google/devtools/build/lib/remote/BUILD | 1 + .../BuildWithoutTheBytesIntegrationTest.java | 78 ++++++++++-- ...ildWithoutTheBytesIntegrationTestBase.java | 115 +++++++++++++++++- .../lib/remote/ByteStreamUploaderTest.java | 3 +- .../build/lib/remote/ChunkerTest.java | 4 +- .../build/lib/remote/GrpcCacheClientTest.java | 2 + .../remote/RemoteActionInputFetcherTest.java | 15 --- .../remote/RemoteExecutionServiceTest.java | 4 +- .../lib/remote/RemoteSpawnCacheTest.java | 6 + .../ActionInputDirectoryTreeTest.java | 25 +++- .../lib/remote/merkletree/MerkleTreeTest.java | 35 +++++- .../devtools/build/lib/remote/util/BUILD | 1 + .../util/FakeSpawnExecutionContext.java | 6 + .../google/devtools/build/remote/worker/BUILD | 1 + 28 files changed, 452 insertions(+), 150 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/LeaseService.java diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index c1c369c0b7acf4..c925b61d36d2dd 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -34,6 +34,7 @@ java_library( "Retrier.java", "AbstractActionInputPrefetcher.java", "ToplevelArtifactsDownloader.java", + "LeaseService.java", ], ), exports = [ @@ -46,13 +47,13 @@ java_library( ":ReferenceCountedChannel", ":Retrier", ":abstract_action_input_prefetcher", + ":lease_service", ":toplevel_artifacts_downloader", "//src/main/java/com/google/devtools/build/lib:build-request-options", "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", - "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", @@ -83,6 +84,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/downloader", "//src/main/java/com/google/devtools/build/lib/remote/grpc", @@ -185,6 +187,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", @@ -220,3 +223,15 @@ java_library( "//third_party:jsr305", ], ) + +java_library( + name = "lease_service", + srcs = ["LeaseService.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/skyframe", + "//third_party:jsr305", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/remote/Chunker.java b/src/main/java/com/google/devtools/build/lib/remote/Chunker.java index ad63d6c80aec9e..ce7372a1c18185 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/Chunker.java +++ b/src/main/java/com/google/devtools/build/lib/remote/Chunker.java @@ -34,7 +34,6 @@ import java.io.PushbackInputStream; import java.util.NoSuchElementException; import java.util.Objects; -import java.util.function.Supplier; /** * 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) { return false; } Chunk other = (Chunk) o; - return other.offset == offset - && other.data.equals(data); + return other.offset == offset && other.data.equals(data); } @Override @@ -102,7 +100,12 @@ public int hashCode() { } } - private final Supplier dataSupplier; + /** A supplier that provide data as {@link InputStream}. */ + public interface ChunkDataSupplier { + InputStream get() throws IOException; + } + + private final ChunkDataSupplier dataSupplier; private final long size; private final int chunkSize; private final Chunk emptyChunk; @@ -117,7 +120,7 @@ public int hashCode() { // lazily on the first call to next(), as opposed to opening it in the constructor or on reset(). private boolean initialized; - Chunker(Supplier dataSupplier, long size, int chunkSize, boolean compressed) { + Chunker(ChunkDataSupplier dataSupplier, long size, int chunkSize, boolean compressed) { this.dataSupplier = checkNotNull(dataSupplier); this.size = size; this.chunkSize = chunkSize; @@ -287,7 +290,7 @@ public static class Builder { private int chunkSize = getDefaultChunkSize(); protected long size; private boolean compressed; - protected Supplier inputStream; + protected ChunkDataSupplier inputStream; @CanIgnoreReturnValue public Builder setInput(byte[] data) { @@ -310,14 +313,7 @@ public Builder setInput(long size, InputStream in) { public Builder setInput(long size, Path file) { checkState(inputStream == null); this.size = size; - inputStream = - () -> { - try { - return file.getInputStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + inputStream = file::getInputStream; return this; } @@ -326,30 +322,16 @@ public Builder setInput(long size, ActionInput actionInput, Path execRoot) { checkState(inputStream == null); this.size = size; if (actionInput instanceof VirtualActionInput) { - inputStream = - () -> { - try { - return ((VirtualActionInput) actionInput).getBytes().newInput(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + inputStream = () -> ((VirtualActionInput) actionInput).getBytes().newInput(); } else { - inputStream = - () -> { - try { - return ActionInputHelper.toInputPath(actionInput, execRoot).getInputStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + inputStream = () -> ActionInputHelper.toInputPath(actionInput, execRoot).getInputStream(); } return this; } @CanIgnoreReturnValue @VisibleForTesting - protected final Builder setInputSupplier(Supplier inputStream) { + protected final Builder setInputSupplier(ChunkDataSupplier inputStream) { this.inputStream = inputStream; return this; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java b/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java new file mode 100644 index 00000000000000..8479d787c99dc0 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java @@ -0,0 +1,76 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote; + +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionCacheUtils; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ActionLookupData; +import com.google.devtools.build.lib.actions.ActionLookupValue; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.cache.ActionCache; +import com.google.devtools.build.skyframe.MemoizingEvaluator; +import java.util.HashMap; +import java.util.Set; +import javax.annotation.Nullable; + +/** A lease service that manages the lease of remote blobs. */ +public class LeaseService { + private final MemoizingEvaluator memoizingEvaluator; + @Nullable private final ActionCache actionCache; + + public LeaseService(MemoizingEvaluator memoizingEvaluator, @Nullable ActionCache actionCache) { + this.memoizingEvaluator = memoizingEvaluator; + this.actionCache = actionCache; + } + + /** Clean up internal state when files are evicted from remote CAS. */ + public void handleMissingInputs(Set missingActionInputs) { + if (missingActionInputs.isEmpty()) { + return; + } + + var actions = new HashMap(); + + try { + for (ActionInput actionInput : missingActionInputs) { + if (actionInput instanceof Artifact.DerivedArtifact) { + Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) actionInput; + ActionLookupData actionLookupData = output.getGeneratingActionKey(); + var actionLookupValue = + memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey()); + if (actionLookupValue instanceof ActionLookupValue) { + Action action = + ((ActionLookupValue) actionLookupValue) + .getAction(actionLookupData.getActionIndex()); + actions.put(actionLookupData, action); + } + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + + if (!actions.isEmpty()) { + var actionKeys = actions.keySet(); + memoizingEvaluator.delete(key -> key instanceof ActionLookupData && actionKeys.contains(key)); + + if (actionCache != null) { + for (var action : actions.values()) { + ActionCacheUtils.removeCacheEntry(actionCache, action); + } + } + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 2f7099a1730fe5..b576f72d4a528f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -64,6 +64,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionRequirements; @@ -371,10 +372,14 @@ private MerkleTree buildInputMerkleTree( spawn, context, (Object nodeKey, InputWalker walker) -> { - subMerkleTrees.add(buildMerkleTreeVisitor(nodeKey, walker, metadataProvider)); + subMerkleTrees.add( + buildMerkleTreeVisitor( + nodeKey, walker, metadataProvider, context.getPathResolver())); }); if (!outputDirMap.isEmpty()) { - subMerkleTrees.add(MerkleTree.build(outputDirMap, metadataProvider, execRoot, digestUtil)); + subMerkleTrees.add( + MerkleTree.build( + outputDirMap, metadataProvider, execRoot, context.getPathResolver(), digestUtil)); } return MerkleTree.merge(subMerkleTrees, digestUtil); } else { @@ -394,16 +399,20 @@ private MerkleTree buildInputMerkleTree( toolSignature == null ? ImmutableSet.of() : toolSignature.toolInputs, context.getMetadataProvider(), execRoot, + context.getPathResolver(), digestUtil); } } private MerkleTree buildMerkleTreeVisitor( - Object nodeKey, InputWalker walker, MetadataProvider metadataProvider) + Object nodeKey, + InputWalker walker, + MetadataProvider metadataProvider, + ArtifactPathResolver artifactPathResolver) throws IOException, ForbiddenActionInputException { MerkleTree result = merkleTreeCache.getIfPresent(nodeKey); if (result == null) { - result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider); + result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider, artifactPathResolver); merkleTreeCache.put(nodeKey, result); } return result; @@ -411,14 +420,23 @@ private MerkleTree buildMerkleTreeVisitor( @VisibleForTesting public MerkleTree uncachedBuildMerkleTreeVisitor( - InputWalker walker, MetadataProvider metadataProvider) + InputWalker walker, + MetadataProvider metadataProvider, + ArtifactPathResolver artifactPathResolver) throws IOException, ForbiddenActionInputException { ConcurrentLinkedQueue subMerkleTrees = new ConcurrentLinkedQueue<>(); subMerkleTrees.add( - MerkleTree.build(walker.getLeavesInputMapping(), metadataProvider, execRoot, digestUtil)); + MerkleTree.build( + walker.getLeavesInputMapping(), + metadataProvider, + execRoot, + artifactPathResolver, + digestUtil)); walker.visitNonLeaves( (Object subNodeKey, InputWalker subWalker) -> { - subMerkleTrees.add(buildMerkleTreeVisitor(subNodeKey, subWalker, metadataProvider)); + subMerkleTrees.add( + buildMerkleTreeVisitor( + subNodeKey, subWalker, metadataProvider, artifactPathResolver)); }); return MerkleTree.merge(subMerkleTrees, digestUtil); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 9d567bf755a2c6..6f1bfbed7ba473 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -957,9 +957,13 @@ public ActionInput getActionInput(Path path) { }); env.getEventBus().register(toplevelArtifactsDownloader); + var leaseService = + new LeaseService( + env.getSkyframeExecutor().getEvaluator(), + env.getBlazeWorkspace().getPersistentActionCache()); + remoteOutputService.setActionInputFetcher(actionInputFetcher); - remoteOutputService.setMemoizingEvaluator(env.getSkyframeExecutor().getEvaluator()); - remoteOutputService.setActionCache(env.getBlazeWorkspace().getPersistentActionCache()); + remoteOutputService.setLeaseService(leaseService); env.getEventBus().register(remoteOutputService); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index f6c5f138a89dd0..2ea7a0f2aa9219 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -20,15 +20,10 @@ import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.ActionCacheUtils; -import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputMap; -import com.google.devtools.build.lib.actions.ActionLookupData; -import com.google.devtools.build.lib.actions.ActionLookupValue; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; -import com.google.devtools.build.lib.actions.cache.ActionCache; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent; @@ -40,10 +35,8 @@ import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.skyframe.MemoizingEvaluator; import com.google.devtools.build.skyframe.SkyFunction.Environment; import java.io.IOException; -import java.util.HashMap; import java.util.Map; import java.util.UUID; import javax.annotation.Nullable; @@ -52,19 +45,14 @@ public class RemoteOutputService implements OutputService { @Nullable private RemoteActionInputFetcher actionInputFetcher; - @Nullable MemoizingEvaluator memoizingEvaluator; - @Nullable ActionCache actionCache; + @Nullable private LeaseService leaseService; void setActionInputFetcher(RemoteActionInputFetcher actionInputFetcher) { this.actionInputFetcher = Preconditions.checkNotNull(actionInputFetcher, "actionInputFetcher"); } - void setMemoizingEvaluator(MemoizingEvaluator memoizingEvaluator) { - this.memoizingEvaluator = memoizingEvaluator; - } - - void setActionCache(ActionCache actionCache) { - this.actionCache = actionCache; + void setLeaseService(LeaseService leaseService) { + this.leaseService = leaseService; } @Override @@ -128,44 +116,8 @@ public void finalizeBuild(boolean buildSuccessful) { @Subscribe public void onExecutionPhaseCompleteEvent(ExecutionPhaseCompleteEvent event) { - processMissingInputs(); - } - - private void processMissingInputs() { - if (memoizingEvaluator == null || actionInputFetcher == null) { - return; - } - - var actions = new HashMap(); - - try { - for (ActionInput actionInput : actionInputFetcher.getMissingActionInputs()) { - if (actionInput instanceof Artifact.DerivedArtifact) { - Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) actionInput; - ActionLookupData actionLookupData = output.getGeneratingActionKey(); - var actionLookupValue = - memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey()); - if (actionLookupValue instanceof ActionLookupValue) { - Action action = - ((ActionLookupValue) actionLookupValue) - .getAction(actionLookupData.getActionIndex()); - actions.put(actionLookupData, action); - } - } - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - - if (!actions.isEmpty()) { - var actionKeys = actions.keySet(); - memoizingEvaluator.delete(key -> key instanceof ActionLookupData && actionKeys.contains(key)); - - if (actionCache != null) { - for (var action : actions.values()) { - ActionCacheUtils.removeCacheEntry(actionCache, action); - } - } + if (leaseService != null && actionInputFetcher != null) { + leaseService.handleMissingInputs(actionInputFetcher.getMissingActionInputs()); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 5b6622fbb0023d..0f7bb077577efa 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -557,7 +557,11 @@ private SpawnResult handleError( catastrophe = true; } else if (remoteCacheFailed) { status = Status.REMOTE_CACHE_FAILED; - detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_FAILED; + if (remoteOptions.useNewExitCodeForLostInputs) { + detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_EVICTED; + } else { + detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_FAILED; + } catastrophe = false; } else { status = Status.EXECUTION_FAILED; diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD index e6279c38a989ff..36e0a515df1eb5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD @@ -10,14 +10,25 @@ filegroup( visibility = ["//src:__subpackages__"], ) +java_library( + name = "cache_not_found_exception", + srcs = ["CacheNotFoundException.java"], + deps = [ + "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", + ], +) + java_library( name = "common", - srcs = glob(["*.java"]), + srcs = glob( + ["*.java"], + exclude = ["CacheNotFoundException.java"], + ), deps = [ + ":cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", - "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD b/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD index 42872e7ea0d1f1..dc506d1ba2ae81 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD @@ -15,7 +15,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//src/main/java/com/google/devtools/build/lib/remote/common", - "//src/main/java/com/google/devtools/build/lib/remote/options", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD index 67e7bd0a4689f4..86b527cd6219f1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD @@ -21,6 +21,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info", "//src/main/java/com/google/devtools/build/lib/authandtls", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:auth", diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java index 3a731981345ede..c9089c187e9091 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; @@ -61,9 +62,11 @@ static DirectoryTree fromActionInputs( SortedMap inputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { - return fromActionInputs(inputs, ImmutableSet.of(), metadataProvider, execRoot, digestUtil); + return fromActionInputs( + inputs, ImmutableSet.of(), metadataProvider, execRoot, artifactPathResolver, digestUtil); } static DirectoryTree fromActionInputs( @@ -71,11 +74,13 @@ static DirectoryTree fromActionInputs( Set toolInputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { Map tree = new HashMap<>(); int numFiles = - buildFromActionInputs(inputs, toolInputs, metadataProvider, execRoot, digestUtil, tree); + buildFromActionInputs( + inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil, tree); return new DirectoryTree(tree, numFiles); } @@ -135,6 +140,7 @@ private static int buildFromActionInputs( Set toolInputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil, Map tree) throws IOException { @@ -164,7 +170,7 @@ private static int buildFromActionInputs( case REGULAR_FILE: { Digest d = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); - Path inputPath = ActionInputHelper.toInputPath(input, execRoot); + Path inputPath = artifactPathResolver.toPath(input); boolean childAdded = currDir.addChild( FileNode.createExecutable( @@ -176,7 +182,13 @@ private static int buildFromActionInputs( SortedMap directoryInputs = explodeDirectory(input.getExecPath(), execRoot); return buildFromActionInputs( - directoryInputs, toolInputs, metadataProvider, execRoot, digestUtil, tree); + directoryInputs, + toolInputs, + metadataProvider, + execRoot, + artifactPathResolver, + digestUtil, + tree); case SYMLINK: { @@ -185,7 +197,7 @@ private static int buildFromActionInputs( "Encountered symlink input '%s', but all source symlinks should have been" + " resolved by SkyFrame. This is a bug.", path); - Path inputPath = ActionInputHelper.toInputPath(input, execRoot); + Path inputPath = artifactPathResolver.toPath(input); boolean childAdded = currDir.addChild( new SymlinkNode( diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java index 5af8d982ba3a00..1179ceca87462c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java @@ -30,6 +30,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -222,11 +223,13 @@ public static MerkleTree build( SortedMap inputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) { DirectoryTree tree = - DirectoryTreeBuilder.fromActionInputs(inputs, metadataProvider, execRoot, digestUtil); + DirectoryTreeBuilder.fromActionInputs( + inputs, metadataProvider, execRoot, artifactPathResolver, digestUtil); return build(tree, digestUtil); } } @@ -247,12 +250,13 @@ public static MerkleTree build( Set toolInputs, MetadataProvider metadataProvider, Path execRoot, + ArtifactPathResolver artifactPathResolver, DigestUtil digestUtil) throws IOException { try (SilentCloseable c = Profiler.instance().profile("MerkleTree.build(ActionInput)")) { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - inputs, toolInputs, metadataProvider, execRoot, digestUtil); + inputs, toolInputs, metadataProvider, execRoot, artifactPathResolver, digestUtil); return build(tree, digestUtil); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD index 0fc286112da896..e01dc07b7fdf74 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD @@ -24,6 +24,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/remote:ExecutionStatusException", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index c9cc73b57da2b8..fae2d964f9ef04 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -491,6 +491,20 @@ public void downloadFile_onInterrupt_deletePartialDownloadedFile() throws Except assertThat(tempPathGenerator.getTempDir().getDirectoryEntries()).isEmpty(); } + @Test + public void missingInputs_addedToList() { + Map metadata = new HashMap<>(); + Map cas = new HashMap<>(); + Artifact a = createRemoteArtifact("file", "hello world", metadata, /* cas= */ null); + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); + + assertThrows( + Exception.class, () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider))); + + assertThat(prefetcher.getMissingActionInputs()).contains(a); + } + protected static void wait(ListenableFuture future) throws IOException, ExecException, InterruptedException { try { @@ -505,7 +519,7 @@ protected static void wait(ListenableFuture future) } throw new IOException(e); } catch (InterruptedException e) { - future.cancel(/*mayInterruptIfRunning=*/ true); + future.cancel(/* mayInterruptIfRunning= */ true); throw e; } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index 477813978b5f03..be53291e5bebec 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -75,6 +75,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote:abstract_action_input_prefetcher", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/grpc", "//src/main/java/com/google/devtools/build/lib/remote/http", diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index 94ef104b381cef..25b5c3cf33c0bb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -86,7 +86,8 @@ protected ImmutableList getSpawnModules() { } @Override - protected void assertOutputEquals(String realContent, String expectedContent) throws Exception { + protected void assertOutputEquals(String realContent, String expectedContent, boolean isLocal) + throws Exception { assertThat(realContent).isEqualTo(expectedContent); } @@ -95,6 +96,11 @@ protected void assertOutputContains(String content, String contains) throws Exce assertThat(content).contains(contains); } + @Override + protected void evictAllBlobs() throws Exception { + worker.restart(); + } + @After public void tearDown() throws IOException { if (worker != null) { @@ -418,7 +424,7 @@ public void symlinkToNestedDirectory() throws Exception { } @Test - public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { + public void remoteCacheEvictBlobs_whenPrefetchingInput_exitWithCode39() throws Exception { // Arrange: Prepare workspace and populate remote cache write( "a/BUILD", @@ -453,7 +459,7 @@ public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { assertOutputDoesNotExist("a/foo.out"); // Act: Evict blobs from remote cache and do an incremental build - worker.restart(); + evictAllBlobs(); write("a/bar.in", "updated bar"); var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); @@ -468,7 +474,7 @@ public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { } @Test - public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception { + public void remoteCacheEvictBlobs_whenUploadingInput_exitWithCode39() throws Exception { // Arrange: Prepare workspace and populate remote cache write( "a/BUILD", @@ -483,24 +489,71 @@ public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception " srcs = ['foo.out', 'bar.in'],", " outs = ['bar.out'],", " cmd = 'cat $(SRCS) > $@',", - " tags = ['no-remote-exec'],", ")"); write("a/foo.in", "foo"); write("a/bar.in", "bar"); // Populate remote cache + setDownloadAll(); buildTarget("//a:bar"); + waitDownloads(); + var bytes = FileSystemUtils.readContent(getOutputPath("a/foo.out")); + var hashCode = getDigestHashFunction().getHashFunction().hashBytes(bytes); getOutputPath("a/foo.out").delete(); getOutputPath("a/bar.out").delete(); getOutputBase().getRelative("action_cache").deleteTreesBelow(); restartServer(); + addOptions("--incompatible_remote_use_new_exit_code_for_lost_inputs"); // Clean build, foo.out isn't downloaded buildTarget("//a:bar"); assertOutputDoesNotExist("a/foo.out"); + // Act: Evict blobs from remote cache and do an incremental build + evictAllBlobs(); + write("a/bar.in", "updated bar"); + var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Assert: Exit code is 39 + assertThat(error).hasMessageThat().contains(String.format("%s/%s", hashCode, bytes.length)); + assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39); + } + + @Test + public void remoteCacheEvictBlobs_whenUploadingInputFile_incrementalBuildCanContinue() + throws Exception { + // Arrange: Prepare workspace and populate remote cache + write( + "a/BUILD", + "genrule(", + " name = 'foo',", + " srcs = ['foo.in'],", + " outs = ['foo.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")"); + write("a/foo.in", "foo"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + getOutputPath("a/foo.out").delete(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + setDownloadToplevel(); + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out"); + // Evict blobs from remote cache - worker.restart(); + evictAllBlobs(); // trigger build error write("a/bar.in", "updated bar"); @@ -509,13 +562,15 @@ public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception // Act: Do an incremental build without "clean" or "shutdown" buildTarget("//a:bar"); + waitDownloads(); // Assert: target was successfully built assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator()); } @Test - public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() throws Exception { + public void remoteCacheEvictBlobs_whenUploadingInputTree_incrementalBuildCanContinue() + throws Exception { // Arrange: Prepare workspace and populate remote cache write("BUILD"); writeOutputDirRule(); @@ -531,7 +586,6 @@ public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() thr " srcs = ['foo.out', 'bar.in'],", " outs = ['bar.out'],", " cmd = '( ls $(location :foo.out); cat $(location :bar.in) ) > $@',", - " tags = ['no-remote-exec'],", ")"); write("a/manifest", "file-inside"); write("a/bar.in", "bar"); @@ -548,18 +602,22 @@ public void remoteCacheEvictBlobs_treeArtifact_incrementalBuildCanContinue() thr assertOutputDoesNotExist("a/foo.out/file-inside"); // Evict blobs from remote cache - worker.restart(); + evictAllBlobs(); // trigger build error + setDownloadToplevel(); write("a/bar.in", "updated bar"); // Build failed because of remote cache eviction assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); // Act: Do an incremental build without "clean" or "shutdown" buildTarget("//a:bar"); + waitDownloads(); // Assert: target was successfully built assertValidOutputFile( - "a/bar.out", "file-inside" + lineSeparator() + "updated bar" + lineSeparator()); + "a/bar.out", + "file-inside" + lineSeparator() + "updated bar" + lineSeparator(), + /* isLocal= */ true); } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java index 0fd2d2d4639ea8..215df976fd1316 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java @@ -42,11 +42,13 @@ public abstract class BuildWithoutTheBytesIntegrationTestBase extends BuildInteg protected abstract void setDownloadAll(); - protected abstract void assertOutputEquals(String realContent, String expectedContent) - throws Exception; + protected abstract void assertOutputEquals( + String realContent, String expectedContent, boolean isLocal) throws Exception; protected abstract void assertOutputContains(String content, String contains) throws Exception; + protected abstract void evictAllBlobs() throws Exception; + protected void waitDownloads() throws Exception { // Trigger afterCommand of modules so that downloads are waited. runtimeWrapper.newCommand(); @@ -731,6 +733,106 @@ public void incrementalBuild_intermediateOutputModified_rerunGeneratingActions() assertThat(executedAction.getPrimaryOutput().getFilename()).isEqualTo("foo.txt"); } + @Test + public void remoteCacheEvictBlobs_whenPrefetchingInputFile_incrementalBuildCanContinue() + throws Exception { + // Arrange: Prepare workspace and populate remote cache + write( + "a/BUILD", + "genrule(", + " name = 'foo',", + " srcs = ['foo.in'],", + " outs = ['foo.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")"); + write("a/foo.in", "foo"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + getOutputPath("a/foo.out").delete(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out"); + + // Evict blobs from remote cache + evictAllBlobs(); + + // trigger build error + write("a/bar.in", "updated bar"); + addOptions("--strategy_regexp=.*bar=local"); + // Build failed because of remote cache eviction + assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Act: Do an incremental build without "clean" or "shutdown" + buildTarget("//a:bar"); + + // Assert: target was successfully built + assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator()); + } + + @Test + public void remoteCacheEvictBlobs_whenPrefetchingInputTree_incrementalBuildCanContinue() + throws Exception { + // Arrange: Prepare workspace and populate remote cache + write("BUILD"); + writeOutputDirRule(); + write( + "a/BUILD", + "load('//:output_dir.bzl', 'output_dir')", + "output_dir(", + " name = 'foo.out',", + " manifest = ':manifest',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = '( ls $(location :foo.out); cat $(location :bar.in) ) > $@',", + ")"); + write("a/manifest", "file-inside"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + getOutputPath("a/foo.out").deleteTreesBelow(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out/file-inside"); + + // Evict blobs from remote cache + evictAllBlobs(); + + // trigger build error + write("a/bar.in", "updated bar"); + addOptions("--strategy_regexp=.*bar=local"); + // Build failed because of remote cache eviction + assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Act: Do an incremental build without "clean" or "shutdown" + buildTarget("//a:bar"); + + // Assert: target was successfully built + assertValidOutputFile( + "a/bar.out", + "file-inside" + lineSeparator() + "updated bar" + lineSeparator(), + /* isLocal= */ true); + } + protected void assertOutputsDoNotExist(String target) throws Exception { for (Artifact output : getArtifacts(target)) { assertWithMessage( @@ -757,12 +859,17 @@ protected void assertOnlyOutputContent(String target, String filename, String co Artifact output = getOnlyElement(getArtifacts(target)); assertThat(output.getFilename()).isEqualTo(filename); assertThat(output.getPath().exists()).isTrue(); - assertOutputEquals(readContent(output.getPath(), UTF_8), content); + assertOutputEquals(readContent(output.getPath(), UTF_8), content, /* isLocal= */ false); } protected void assertValidOutputFile(String binRelativePath, String content) throws Exception { + assertValidOutputFile(binRelativePath, content, /* isLocal= */ false); + } + + protected void assertValidOutputFile(String binRelativePath, String content, boolean isLocal) + throws Exception { Path output = getOutputPath(binRelativePath); - assertOutputEquals(readContent(output, UTF_8), content); + assertOutputEquals(readContent(output, UTF_8), content, isLocal); assertThat(output.isReadable()).isTrue(); assertThat(output.isWritable()).isFalse(); assertThat(output.isExecutable()).isTrue(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java index ebf15d53f593c6..f0880d2ea9b6eb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java @@ -79,7 +79,6 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Supplier; import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; @@ -1637,7 +1636,7 @@ public void queryWriteStatus( /* Custom Chunker used to track number of open files */ private static class TestChunker extends Chunker { - TestChunker(Supplier dataSupplier, long size, int chunkSize, boolean compressed) { + TestChunker(ChunkDataSupplier dataSupplier, long size, int chunkSize, boolean compressed) { super(dataSupplier, size, chunkSize, compressed); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java b/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java index 6de8758a7d0466..cd4fa6943034c3 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java @@ -18,6 +18,7 @@ import com.github.luben.zstd.Zstd; import com.google.devtools.build.lib.remote.Chunker.Chunk; +import com.google.devtools.build.lib.remote.Chunker.ChunkDataSupplier; import com.google.protobuf.ByteString; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -26,7 +27,6 @@ import java.util.NoSuchElementException; import java.util.Random; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Supplier; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -126,7 +126,7 @@ public void resourcesShouldBeReleased() throws IOException { byte[] data = new byte[] {1, 2}; final AtomicReference in = new AtomicReference<>(); - Supplier supplier = + ChunkDataSupplier supplier = () -> { in.set(Mockito.spy(new ByteArrayInputStream(data))); return in.get(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index 9f7b55148c6330..c71f7166bff510 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -51,6 +51,7 @@ import com.google.common.collect.Maps; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionInputHelper; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.events.NullEventHandler; @@ -109,6 +110,7 @@ public void testVirtualActionInputSupport() throws Exception { ImmutableSortedMap.of(execPath, virtualActionInput), fakeFileCache, execRoot, + ArtifactPathResolver.forExecRoot(execRoot), DIGEST_UTIL); Digest digest = DIGEST_UTIL.compute(virtualActionInput.getBytes().toByteArray()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java index e919392fda5d1b..e82fcfede54211 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java @@ -152,21 +152,6 @@ public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Excepti .contains(String.format("%s/%s", digest.getHash(), digest.getSizeBytes())); } - @Test - public void missingInputs_addedToList() { - Map metadata = new HashMap<>(); - Map cas = new HashMap<>(); - Artifact a = createRemoteArtifact("file", "hello world", metadata, /* cas= */ null); - MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); - AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - - assertThrows( - ExecException.class, - () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider))); - - assertThat(prefetcher.getMissingActionInputs()).contains(a); - } - private RemoteCache newCache( RemoteOptions options, DigestUtil digestUtil, Map cas) { Map cacheEntries = Maps.newHashMapWithExpectedSize(cas.size()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 5145d78b216e4c..e7bf966362aaab 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -1796,14 +1796,14 @@ public void buildMerkleTree_withMemoization_works() throws Exception { // assert first time // Called for: manifests, runfiles, nodeRoot1, nodeFoo1 and nodeBar. - verify(service, times(5)).uncachedBuildMerkleTreeVisitor(any(), any()); + verify(service, times(5)).uncachedBuildMerkleTreeVisitor(any(), any(), any()); // act second time service.buildRemoteAction(spawn2, context2); // assert second time // Called again for: manifests, runfiles, nodeRoot2 and nodeFoo2 but not nodeBar (cached). - verify(service, times(5 + 4)).uncachedBuildMerkleTreeVisitor(any(), any()); + verify(service, times(5 + 4)).uncachedBuildMerkleTreeVisitor(any(), any(), any()); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 05a635dfde09ae..f0a6fefd5eb023 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.MetadataProvider; @@ -151,6 +152,11 @@ public MetadataProvider getMetadataProvider() { return fakeFileCache; } + @Override + public ArtifactPathResolver getPathResolver() { + return ArtifactPathResolver.forExecRoot(execRoot); + } + @Override public ArtifactExpander getArtifactExpander() { throw new UnsupportedOperationException(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java index 6af2ab698c80ff..d70d39a3e1aefe 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; @@ -50,7 +51,11 @@ protected DirectoryTree build(Path... paths) throws IOException { } return DirectoryTreeBuilder.fromActionInputs( - inputFiles, new StaticMetadataProvider(metadata), execRoot, digestUtil); + inputFiles, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); } @Test @@ -63,7 +68,11 @@ public void virtualActionInputShouldWork() throws Exception { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); assertLexicographicalOrder(tree); assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); @@ -108,7 +117,11 @@ public void directoryInputShouldBeExpanded() throws Exception { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); assertLexicographicalOrder(tree); assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); @@ -152,7 +165,11 @@ public void filesShouldBeDeduplicated() throws Exception { DirectoryTree tree = DirectoryTreeBuilder.fromActionInputs( - sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); assertLexicographicalOrder(tree); assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java index af96bb2d46f106..3c01f95354f4b1 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.FileArtifactValue; @@ -72,6 +73,7 @@ public void emptyMerkleTree() throws IOException { Collections.emptySortedMap(), new StaticMetadataProvider(Collections.emptyMap()), execRoot, + ArtifactPathResolver.forExecRoot(execRoot), digestUtil); Digest emptyDigest = digestUtil.compute(new byte[0]); assertThat(tree.getRootDigest()).isEqualTo(emptyDigest); @@ -108,7 +110,12 @@ public void buildMerkleTree() throws IOException { // act MerkleTree tree = - MerkleTree.build(sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree.build( + sortedInputs, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); // assert Digest expectedRootDigest = digestUtil.compute(rootDir); @@ -157,14 +164,32 @@ public void mergeMerkleTrees() throws IOException { MerkleTree treeEmpty = MerkleTree.build( - new TreeMap<>(), new StaticMetadataProvider(metadata), execRoot, digestUtil); + new TreeMap<>(), + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); MerkleTree tree1 = - MerkleTree.build(sortedInputs1, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree.build( + sortedInputs1, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); MerkleTree tree2 = - MerkleTree.build(sortedInputs2, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree.build( + sortedInputs2, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); MerkleTree treeAll = MerkleTree.build( - sortedInputsAll, new StaticMetadataProvider(metadata), execRoot, digestUtil); + sortedInputsAll, + new StaticMetadataProvider(metadata), + execRoot, + ArtifactPathResolver.forExecRoot(execRoot), + digestUtil); // act MerkleTree mergedTreeEmpty = MerkleTree.merge(Arrays.asList(), digestUtil); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD index d5d0bfaa71d13c..d22fbca83e3ad8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/util/BUILD @@ -24,6 +24,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java index 46dc2572a29ae9..8f9e41e31e7a25 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.Spawn; @@ -122,6 +123,11 @@ public SpawnInputExpander getSpawnInputExpander() { return new SpawnInputExpander(execRoot, /* strict= */ false); } + @Override + public ArtifactPathResolver getPathResolver() { + return ArtifactPathResolver.forExecRoot(execRoot); + } + @Override public Duration getTimeout() { return Duration.ZERO; diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD index b0a8dbcc529645..a6309fb6303701 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD @@ -21,6 +21,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/remote/util",