From f7d34f493df79b7b1b179970e3261e89cfba43f7 Mon Sep 17 00:00:00 2001 From: "Ian (Hee) Cha" Date: Tue, 10 Sep 2024 01:45:37 -0700 Subject: [PATCH] [7.4.0] Make HTTP cache works with cache eviction. (#23561) Previously, it's possible for an HTTP cache to delete CAS entries referenced by AC without deleting the AC itself because HTTP cache doesn't understand the relationship between AC and CAS. This could result in permanent build errors because Bazel always trust the AC from remote cache assuming all referenced CAS entries exist. Now, we record the digest of lost blobs before build rewinding, so that during the next build, we can ignore the stale AC and continue with execution. Fixes #18696. RELNOTES: Added support for using a remote cache that evicts blobs and doesn't have AC integrity check (e.g. HTTP cache). PiperOrigin-RevId: 672536163 Change-Id: Ic1271431d28333f6d86e5963542d15a133075157 Commit https://github.com/bazelbuild/bazel/commit/5d815790a0f5007068ed87a757e0f20aa80020ad Co-authored-by: Googler --- .../remote/AbstractActionInputPrefetcher.java | 5 +- .../remote/RemoteActionContextProvider.java | 28 +++-- .../lib/remote/RemoteExecutionCache.java | 2 +- .../lib/remote/RemoteExecutionService.java | 119 +++++++++++++++--- .../build/lib/remote/RemoteModule.java | 21 +++- .../devtools/build/lib/remote/common/BUILD | 1 + .../lib/remote/common/LostInputsEvent.java | 13 +- .../remote/RemoteExecutionServiceTest.java | 4 +- .../lib/remote/RemoteSpawnCacheTest.java | 4 +- .../lib/remote/RemoteSpawnRunnerTest.java | 7 +- ...SpawnRunnerWithGrpcRemoteExecutorTest.java | 4 +- .../remote/remote_execution_http_test.sh | 43 +++++++ src/test/shell/bazel/remote/remote_utils.sh | 4 + .../worker/OnDiskHttpCacheServerHandler.java | 3 + 14 files changed, 220 insertions(+), 38 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index 5c8798164a2bef..cd8c0c97b42c11 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java @@ -536,8 +536,9 @@ private Completable downloadFileNoCheckRx( }) .doOnError( error -> { - if (error instanceof CacheNotFoundException) { - reporter.post(new LostInputsEvent()); + if (error instanceof CacheNotFoundException cacheNotFoundException) { + reporter.post( + new LostInputsEvent(cacheNotFoundException.getMissingDigest())); } })); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 7c7a51f0704ad2..bac348fbf467b1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -16,6 +16,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; +import build.bazel.remote.execution.v2.Digest; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -33,6 +34,7 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; +import java.util.Set; import java.util.concurrent.Executor; import javax.annotation.Nullable; @@ -51,6 +53,7 @@ final class RemoteActionContextProvider { @Nullable private RemoteActionInputFetcher actionInputFetcher; @Nullable private final RemoteOutputChecker remoteOutputChecker; @Nullable private final OutputService outputService; + private final Set knownMissingCasDigests; private RemoteActionContextProvider( Executor executor, @@ -61,7 +64,8 @@ private RemoteActionContextProvider( DigestUtil digestUtil, @Nullable Path logDir, @Nullable RemoteOutputChecker remoteOutputChecker, - @Nullable OutputService outputService) { + @Nullable OutputService outputService, + Set knownMissingCasDigests) { this.executor = executor; this.env = Preconditions.checkNotNull(env, "env"); this.remoteCache = remoteCache; @@ -71,12 +75,14 @@ private RemoteActionContextProvider( this.logDir = logDir; this.remoteOutputChecker = remoteOutputChecker; this.outputService = outputService; + this.knownMissingCasDigests = knownMissingCasDigests; } public static RemoteActionContextProvider createForPlaceholder( CommandEnvironment env, ListeningScheduledExecutorService retryScheduler, - DigestUtil digestUtil) { + DigestUtil digestUtil, + Set knownMissingCasDigests) { return new RemoteActionContextProvider( directExecutor(), env, @@ -86,7 +92,8 @@ public static RemoteActionContextProvider createForPlaceholder( digestUtil, /* logDir= */ null, /* remoteOutputChecker= */ null, - /* outputService= */ null); + /* outputService= */ null, + knownMissingCasDigests); } public static RemoteActionContextProvider createForRemoteCaching( @@ -96,7 +103,8 @@ public static RemoteActionContextProvider createForRemoteCaching( ListeningScheduledExecutorService retryScheduler, DigestUtil digestUtil, @Nullable RemoteOutputChecker remoteOutputChecker, - OutputService outputService) { + OutputService outputService, + Set knownMissingCasDigests) { return new RemoteActionContextProvider( executor, env, @@ -106,7 +114,8 @@ public static RemoteActionContextProvider createForRemoteCaching( digestUtil, /* logDir= */ null, remoteOutputChecker, - checkNotNull(outputService)); + checkNotNull(outputService), + knownMissingCasDigests); } public static RemoteActionContextProvider createForRemoteExecution( @@ -118,7 +127,8 @@ public static RemoteActionContextProvider createForRemoteExecution( DigestUtil digestUtil, Path logDir, @Nullable RemoteOutputChecker remoteOutputChecker, - OutputService outputService) { + OutputService outputService, + Set knownMissingCasDigests) { return new RemoteActionContextProvider( executor, env, @@ -128,7 +138,8 @@ public static RemoteActionContextProvider createForRemoteExecution( digestUtil, logDir, remoteOutputChecker, - checkNotNull(outputService)); + checkNotNull(outputService), + knownMissingCasDigests); } private RemotePathResolver createRemotePathResolver() { @@ -180,7 +191,8 @@ private RemoteExecutionService getRemoteExecutionService() { tempPathGenerator, captureCorruptedOutputsDir, remoteOutputChecker, - outputService); + outputService, + knownMissingCasDigests); env.getEventBus().register(remoteExecutionService); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java index f9f7603af469ed..cec1f33d197b54 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java @@ -191,7 +191,7 @@ private ListenableFuture uploadBlob( // If we get here, the remote input was determined to exist in the remote or disk cache at // some point before action execution, but reported to be missing when querying the remote // for missing action inputs; possibly because it was evicted in the interim. - reporter.post(new LostInputsEvent()); + reporter.post(new LostInputsEvent(digest)); throw new CacheNotFoundException(digest, path.getPathString()); } } catch (IOException e) { 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 31d78973c40fe7..ccefcc55a40d21 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 @@ -74,6 +74,7 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.analysis.platform.PlatformUtils; +import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent; import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; @@ -88,6 +89,7 @@ import com.google.devtools.build.lib.remote.RemoteExecutionService.ActionResultMetadata.SymlinkMetadata; import com.google.devtools.build.lib.remote.Scrubber.SpawnScrubber; import com.google.devtools.build.lib.remote.common.BulkTransferException; +import com.google.devtools.build.lib.remote.common.LostInputsEvent; import com.google.devtools.build.lib.remote.common.OperationObserver; import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException; import com.google.devtools.build.lib.remote.common.ProgressStatusListener; @@ -189,6 +191,7 @@ public class RemoteExecutionService { private final OutputService outputService; @Nullable private final Scrubber scrubber; + private final Set knownMissingCasDigests; public RemoteExecutionService( Executor executor, @@ -205,7 +208,8 @@ public RemoteExecutionService( TempPathGenerator tempPathGenerator, @Nullable Path captureCorruptedOutputsDir, @Nullable RemoteOutputChecker remoteOutputChecker, - OutputService outputService) { + OutputService outputService, + Set knownMissingCasDigests) { this.reporter = reporter; this.verboseFailures = verboseFailures; this.execRoot = execRoot; @@ -231,6 +235,7 @@ public RemoteExecutionService( this.scheduler = Schedulers.from(executor, /* interruptibleWorker= */ true); this.remoteOutputChecker = remoteOutputChecker; this.outputService = outputService; + this.knownMissingCasDigests = knownMissingCasDigests; } private Command buildCommand( @@ -648,6 +653,7 @@ public static class RemoteActionResult { private final ActionResult actionResult; @Nullable private final ExecuteResponse executeResponse; @Nullable private final String cacheName; + @Nullable private ActionResultMetadata metadata; /** Creates a new {@link RemoteActionResult} instance from a cached result. */ public static RemoteActionResult createFromCache(CachedActionResult cachedActionResult) { @@ -688,8 +694,20 @@ public List getOutputDirectories() { return actionResult.getOutputDirectoriesList(); } - public int getOutputDirectoriesCount() { - return actionResult.getOutputDirectoriesCount(); + public ActionResultMetadata getOrParseActionResultMetadata( + RemoteCache remoteCache, + DigestUtil digestUtil, + RemoteActionExecutionContext context, + RemotePathResolver remotePathResolver) + throws IOException, InterruptedException { + if (metadata == null) { + try (SilentCloseable c = Profiler.instance().profile("Remote.parseActionResultMetadata")) { + metadata = + parseActionResultMetadata( + remoteCache, digestUtil, context, actionResult, remotePathResolver); + } + } + return metadata; } public List getOutputDirectorySymlinks() { @@ -783,7 +801,53 @@ public RemoteActionResult lookupCache(RemoteAction action) return null; } - return RemoteActionResult.createFromCache(cachedActionResult); + var result = RemoteActionResult.createFromCache(cachedActionResult); + + // We only add digests to `knownMissingCasDigests` when LostInputsEvent occurs which will cause + // the build to abort and rewind, so there is no data race here. This allows us to avoid the + // check until cache eviction happens. + if (!knownMissingCasDigests.isEmpty()) { + var metadata = + result.getOrParseActionResultMetadata( + remoteCache, + digestUtil, + action.getRemoteActionExecutionContext(), + action.getRemotePathResolver()); + + // If we already know digests referenced by this AC is missing from remote cache, ignore it so + // that we can fall back to execution. This could happen when the remote cache is an HTTP + // cache, or doesn't implement AC integrity check. + // + // See https://github.com/bazelbuild/bazel/issues/18696. + if (updateKnownMissingCasDigests(knownMissingCasDigests, metadata)) { + return null; + } + } + + return result; + } + + /** + * Removes digests referenced by {@code metadata} from {@code knownMissingCasDigests} and returns + * whether any were removed + */ + private static boolean updateKnownMissingCasDigests( + Set knownMissingCasDigests, ActionResultMetadata metadata) { + // Using `remove` below because we assume the missing blob will be uploaded afterwards. + var result = false; + for (var file : metadata.files()) { + if (knownMissingCasDigests.remove(file.digest())) { + result = true; + } + } + for (var entry : metadata.directories()) { + for (var file : entry.getValue().files()) { + if (knownMissingCasDigests.remove(file.digest())) { + result = true; + } + } + } + return result; } private ListenableFuture downloadFile( @@ -995,7 +1059,7 @@ public Collection symlinks() { } } - private DirectoryMetadata parseDirectory( + private static DirectoryMetadata parseDirectory( Path parent, Directory dir, Map childDirectoriesMap) { ImmutableList.Builder filesBuilder = ImmutableList.builder(); for (FileNode file : dir.getFilesList()) { @@ -1023,16 +1087,18 @@ private DirectoryMetadata parseDirectory( return new DirectoryMetadata(filesBuilder.build(), symlinksBuilder.build()); } - ActionResultMetadata parseActionResultMetadata( + static ActionResultMetadata parseActionResultMetadata( + RemoteCache remoteCache, + DigestUtil digestUtil, RemoteActionExecutionContext context, - RemoteActionResult result, + ActionResult result, RemotePathResolver remotePathResolver) throws IOException, InterruptedException { checkNotNull(remoteCache, "remoteCache can't be null"); Map> dirMetadataDownloads = Maps.newHashMapWithExpectedSize(result.getOutputDirectoriesCount()); - for (OutputDirectory dir : result.getOutputDirectories()) { + for (OutputDirectory dir : result.getOutputDirectoriesList()) { var outputPath = encodeBytestringUtf8(dir.getPath()); dirMetadataDownloads.put( remotePathResolver.outputPathToLocalPath(outputPath), @@ -1059,7 +1125,7 @@ ActionResultMetadata parseActionResultMetadata( } ImmutableMap.Builder files = ImmutableMap.builder(); - for (OutputFile outputFile : result.getOutputFiles()) { + for (OutputFile outputFile : result.getOutputFilesList()) { Path localPath = remotePathResolver.outputPathToLocalPath(encodeBytestringUtf8(outputFile.getPath())); files.put( @@ -1070,9 +1136,9 @@ ActionResultMetadata parseActionResultMetadata( var symlinkMap = new HashMap(); var outputSymlinks = Iterables.concat( - result.getOutputFileSymlinks(), - result.getOutputDirectorySymlinks(), - result.getOutputSymlinks()); + result.getOutputFileSymlinksList(), + result.getOutputDirectorySymlinksList(), + result.getOutputSymlinksList()); for (var symlink : outputSymlinks) { var localPath = remotePathResolver.outputPathToLocalPath(encodeBytestringUtf8(symlink.getPath())); @@ -1132,10 +1198,9 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re context = context.withReadCachePolicy(context.getReadCachePolicy().addRemoteCache()); } - ActionResultMetadata metadata; - try (SilentCloseable c = Profiler.instance().profile("Remote.parseActionResultMetadata")) { - metadata = parseActionResultMetadata(context, result, action.getRemotePathResolver()); - } + ActionResultMetadata metadata = + result.getOrParseActionResultMetadata( + remoteCache, digestUtil, context, action.getRemotePathResolver()); // The expiration time for remote cache entries. var expireAtEpochMilli = Instant.now().plus(remoteOptions.remoteCacheTtl).toEpochMilli(); @@ -1205,6 +1270,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re if (realToTmpPath.containsKey(file.path)) { continue; } + if (shouldDownload(result, file.path.relativeTo(execRoot))) { Path tmpPath = tempPathGenerator.generateTempPath(); realToTmpPath.put(file.path, tmpPath); @@ -1308,6 +1374,12 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re } } + if (result.executeResponse != null && !knownMissingCasDigests.isEmpty()) { + // A succeeded execution uploads outputs to CAS. Refresh our knowledge about missing + // digests. + var unused = updateKnownMissingCasDigests(knownMissingCasDigests, metadata); + } + // When downloading outputs from just remotely executed action, the action result comes from // Execution response which means, if disk cache is enabled, action result hasn't been // uploaded to it. Upload action result to disk cache here so next build could hit it. @@ -1591,6 +1663,20 @@ public void onBuildInterrupted(BuildInterruptedEvent event) { buildInterrupted.set(true); } + @Subscribe + public void onBuildComplete(BuildCompleteEvent event) { + if (event.getResult().getSuccess()) { + // If build succeeded, clear knownMissingCasDigests in case there are missing digests from + // other targets from previous builds which are not relevant anymore. + knownMissingCasDigests.clear(); + } + } + + @Subscribe + public void onLostInputs(LostInputsEvent event) { + knownMissingCasDigests.add(event.getMissingDigest()); + } + /** * Shuts the service down. Wait for active network I/O to finish but new requests are rejected. */ @@ -1625,7 +1711,6 @@ public void shutdown() { } void report(Event evt) { - synchronized (this) { if (reportedErrors.contains(evt.getMessage())) { return; 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 6df0c132177592..f4811b911e51fa 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 @@ -16,6 +16,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; +import build.bazel.remote.execution.v2.Digest; import build.bazel.remote.execution.v2.DigestFunction; import com.github.benmanes.caffeine.cache.Cache; import com.google.auth.Credentials; @@ -26,6 +27,7 @@ import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -113,6 +115,7 @@ import java.nio.channels.ClosedChannelException; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.LinkedBlockingQueue; @@ -127,6 +130,8 @@ public final class RemoteModule extends BlazeModule { private final ListeningScheduledExecutorService retryScheduler = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); + private final Set knownMissingCasDigests = Sets.newConcurrentHashSet(); + @Nullable private AsynchronousMessageOutputStream rpcLogFile; @Nullable private ExecutorService executorService; @Nullable private RemoteActionContextProvider actionContextProvider; @@ -249,7 +254,8 @@ private void initHttpAndDiskCache( /* retryScheduler= */ null, digestUtil, remoteOutputChecker, - outputService); + outputService, + knownMissingCasDigests); } @Override @@ -272,6 +278,10 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { Preconditions.checkState(remoteOutputChecker == null, "remoteOutputChecker must be null"); Preconditions.checkState(outputService == null, "remoteOutputService must be null"); + if ("clean".equals(env.getCommandName())) { + knownMissingCasDigests.clear(); + } + RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class); if (remoteOptions == null) { // Quit if no supported command is being used. See getCommandOptions for details. @@ -328,7 +338,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { if (!enableDiskCache && !enableHttpCache && !enableGrpcCache && !enableRemoteExecution) { // Quit if no remote caching or execution was enabled. actionContextProvider = - RemoteActionContextProvider.createForPlaceholder(env, retryScheduler, digestUtil); + RemoteActionContextProvider.createForPlaceholder( + env, retryScheduler, digestUtil, knownMissingCasDigests); return; } @@ -643,7 +654,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { digestUtil, logDir, remoteOutputChecker, - outputService); + outputService, + knownMissingCasDigests); repositoryRemoteExecutorFactoryDelegate.init( new RemoteRepositoryRemoteExecutorFactory( remoteCache, @@ -680,7 +692,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { retryScheduler, digestUtil, remoteOutputChecker, - outputService); + outputService, + knownMissingCasDigests); } buildEventArtifactUploaderFactoryDelegate.init( 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 6b8390e8a06fec..45a01eca593ab8 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 @@ -35,6 +35,7 @@ java_library( srcs = ["LostInputsEvent.java"], deps = [ "//src/main/java/com/google/devtools/build/lib/events", + "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/LostInputsEvent.java b/src/main/java/com/google/devtools/build/lib/remote/common/LostInputsEvent.java index b770ff263c5aed..ded8760998083e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/LostInputsEvent.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/LostInputsEvent.java @@ -13,10 +13,21 @@ // limitations under the License. package com.google.devtools.build.lib.remote.common; +import build.bazel.remote.execution.v2.Digest; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; /** * An event sent when an input to an action was previously determined to exist remotely, but has * since been evicted. */ -public class LostInputsEvent implements Postable {} +public class LostInputsEvent implements Postable { + private final Digest missingDigest; + + public LostInputsEvent(Digest missingDigest) { + this.missingDigest = missingDigest; + } + + public Digest getMissingDigest() { + return missingDigest; + } +} 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 653e20701c6989..6c7a2bb7b68c2d 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 @@ -54,6 +54,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.SettableFuture; @@ -2514,7 +2515,8 @@ private RemoteExecutionService newRemoteExecutionService(RemoteOptions remoteOpt tempPathGenerator, null, remoteOutputChecker, - outputService); + outputService, + Sets.newConcurrentHashSet()); } private RunfilesSupplier createRunfilesSupplier(String root, Collection artifacts) { 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 5f89e194f55f6b..26a1621525d177 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 @@ -39,6 +39,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionContext; @@ -258,7 +259,8 @@ private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { tempPathGenerator, /* captureCorruptedOutputsDir= */ null, DUMMY_REMOTE_OUTPUT_CHECKER, - mock(OutputService.class))); + mock(OutputService.class), + Sets.newConcurrentHashSet())); return new RemoteSpawnCache( execRoot, options, /* verboseFailures= */ true, service, digestUtil); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index ce9c71a396f5db..e7994fc4803cb8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -46,6 +46,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.ListeningScheduledExecutorService; @@ -1149,7 +1150,8 @@ private void testParamFilesAreMaterializedForFlag(String flag) throws Exception tempPathGenerator, /* captureCorruptedOutputsDir= */ null, remoteOutputChecker, - mock(OutputService.class)); + mock(OutputService.class), + Sets.newConcurrentHashSet()); RemoteSpawnRunner runner = new RemoteSpawnRunner( execRoot, @@ -1685,7 +1687,8 @@ private RemoteSpawnRunner newSpawnRunner( tempPathGenerator, /* captureCorruptedOutputsDir= */ null, remoteOutputChecker, - mock(OutputService.class))); + mock(OutputService.class), + Sets.newConcurrentHashSet())); return new RemoteSpawnRunner( execRoot, diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java index 992253f65199a5..13ae1073e68f74 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java @@ -60,6 +60,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; @@ -349,7 +350,8 @@ public int maxConcurrency() { tempPathGenerator, /* captureCorruptedOutputsDir= */ null, remoteOutputChecker, - mock(OutputService.class)); + mock(OutputService.class), + Sets.newConcurrentHashSet()); client = new RemoteSpawnRunner( execRoot, diff --git a/src/test/shell/bazel/remote/remote_execution_http_test.sh b/src/test/shell/bazel/remote/remote_execution_http_test.sh index 5b50b3c1bebcd6..a47b478b97bdee 100755 --- a/src/test/shell/bazel/remote/remote_execution_http_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_http_test.sh @@ -536,4 +536,47 @@ EOF expect_log "WARNING: Credentials are transmitted in plaintext" } +function test_remote_http_cache_with_missing_cas_referenced_by_ac() { + mkdir -p a + cat > a/BUILD < \$@", +) +genrule( + name = 'foo', + srcs = [":bar"], + outs = ["foo.txt"], + cmd = "echo \"foo\" > \$@", + tags = ["local"], +) +EOF + + # Populate the cache + bazel build \ + --remote_cache=http://localhost:${http_port} \ + //a:foo \ + || fail "Failed to build //a:foo with remote cache" + remote_cas_files="$(count_remote_cas_files)" + # bar.txt, stdout and stderr for action 'bar' + [[ "$remote_cas_files" == 3 ]] || fail "Expected 3 remote cas entries, not $remote_cas_files" + + # Delete blobs from CAS + delete_remote_cas_files + + bazel clean + bazel build \ + --remote_cache=http://localhost:${http_port} \ + --experimental_remote_cache_eviction_retries=1 \ + //a:foo \ + || fail "Failed to build //a:foo with remote cache" + + # Assert that AC is ignored because blobs are missing from CAS, and new output + # is uploaded to CAS. + expect_not_log 'remote cache hit' + remote_cas_files="$(count_remote_cas_files)" + [[ "$remote_cas_files" == 3 ]] || fail "Expected 3 remote cas entries, not $remote_cas_files" +} + run_suite "Remote execution and remote cache tests" diff --git a/src/test/shell/bazel/remote/remote_utils.sh b/src/test/shell/bazel/remote/remote_utils.sh index 0facf1b03beab4..b6ade942cc6306 100644 --- a/src/test/shell/bazel/remote/remote_utils.sh +++ b/src/test/shell/bazel/remote/remote_utils.sh @@ -105,3 +105,7 @@ function remote_cas_file_exist() { function append_remote_cas_files() { find "$cas_path/cas" -type f >> $1 } + +function delete_remote_cas_files() { + rm -rf "$cas_path/cas" +} \ No newline at end of file diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/OnDiskHttpCacheServerHandler.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/OnDiskHttpCacheServerHandler.java index b9a24a0daed1b2..59c6790a471a62 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/OnDiskHttpCacheServerHandler.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/OnDiskHttpCacheServerHandler.java @@ -22,6 +22,7 @@ import io.netty.channel.ChannelHandler.Sharable; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.FileNotFoundException; import java.io.IOException; import javax.annotation.Nullable; @@ -51,6 +52,8 @@ protected byte[] readFromCache(String uri) throws IOException { var in = path.getInputStream()) { ByteStreams.copy(in, out); return out.toByteArray(); + } catch (FileNotFoundException e) { + return null; } }