From b58f1b3d4c675a1cd4f083bac9bf97720942da9b Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Fri, 27 Jan 2023 14:06:27 +0100 Subject: [PATCH] Implement lease service --- .../build/lib/actions/ActionCacheChecker.java | 31 +- .../cache/CompactPersistentActionCache.java | 2 + .../google/devtools/build/lib/remote/BUILD | 1 + .../ByteStreamBuildEventArtifactUploader.java | 5 +- .../build/lib/remote/GrpcCacheClient.java | 2 +- .../lib/remote/RemoteActionFileSystem.java | 8 +- .../build/lib/remote/RemoteCache.java | 5 +- .../lib/remote/RemoteExecutionCache.java | 3 +- .../lib/remote/RemoteExecutionService.java | 3 +- .../build/lib/remote/RemoteLeaseService.java | 342 ++++++++++++++++++ .../build/lib/remote/RemoteModule.java | 20 + .../build/lib/remote/RemoteOutputService.java | 55 ++- .../build/lib/remote/UploadManifest.java | 3 +- .../remote/common/MissingDigestsFinder.java | 7 +- .../remote/disk/DiskAndRemoteCacheClient.java | 38 +- .../lib/remote/disk/DiskCacheClient.java | 16 +- .../lib/remote/http/HttpCacheClient.java | 5 +- .../remote/options/CommonRemoteOptions.java | 50 +++ .../lib/remote/options/RemoteOptions.java | 23 +- .../build/lib/runtime/BlazeWorkspace.java | 2 +- .../lib/skyframe/FilesystemValueChecker.java | 48 ++- .../skyframe/SequencedSkyframeExecutor.java | 5 +- .../lib/skyframe/SkyframeActionExecutor.java | 18 +- .../devtools/build/lib/vfs/LeaseService.java | 5 + .../devtools/build/lib/vfs/OutputService.java | 5 + src/main/protobuf/BUILD | 10 + src/main/protobuf/remote_lease.proto | 30 ++ .../lib/actions/ActionCacheCheckerTest.java | 21 +- ...eStreamBuildEventArtifactUploaderTest.java | 15 +- .../remote/RemoteActionFileSystemTest.java | 3 +- .../build/lib/remote/RemoteCacheTest.java | 27 +- .../remote/RemoteExecutionServiceTest.java | 13 +- .../lib/remote/RemoteLeaseServiceTest.java | 157 ++++++++ .../lib/remote/options/RemoteOptionsTest.java | 5 +- .../lib/remote/util/InMemoryCacheClient.java | 2 +- .../skyframe/FilesystemValueCheckerTest.java | 66 ++-- 36 files changed, 927 insertions(+), 124 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/RemoteLeaseService.java create mode 100644 src/main/java/com/google/devtools/build/lib/vfs/LeaseService.java create mode 100644 src/main/protobuf/remote_lease.proto create mode 100644 src/test/java/com/google/devtools/build/lib/remote/RemoteLeaseServiceTest.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index 5d76e5220f73c2..603c03c26175a2 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java @@ -42,10 +42,12 @@ import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.skyframe.TreeArtifactValue.ArchivedRepresentation; import com.google.devtools.build.lib.vfs.OutputPermissions; +import com.google.devtools.build.lib.vfs.LeaseService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.FileNotFoundException; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -199,11 +201,18 @@ private static boolean validateArtifacts( NestedSet actionInputs, MetadataHandler metadataHandler, boolean checkOutput, - @Nullable CachedOutputMetadata cachedOutputMetadata) { + @Nullable CachedOutputMetadata cachedOutputMetadata, + @Nullable LeaseService leaseService) { Map mdMap = new HashMap<>(); if (checkOutput) { for (Artifact artifact : action.getOutputs()) { FileArtifactValue metadata = getCachedMetadata(cachedOutputMetadata, artifact); + if (leaseService != null && metadata != null && metadata.isRemote()) { + if (!leaseService.isAlive( + metadata.getDigest(), metadata.getSize(), metadata.getLocationIndex())) { + metadata = null; + } + } if (metadata == null) { metadata = getMetadataMaybe(metadataHandler, artifact); } @@ -434,7 +443,8 @@ public Token getTokenIfNeedToExecute( MetadataHandler metadataHandler, ArtifactExpander artifactExpander, Map remoteDefaultPlatformProperties, - boolean loadCachedOutputMetadata) + boolean loadCachedOutputMetadata, + @Nullable LeaseService leaseService) throws InterruptedException { // TODO(bazel-team): (2010) For RunfilesAction/SymlinkAction and similar actions that // produce only symlinks we should not check whether inputs are valid at all - all that matters @@ -494,7 +504,8 @@ public Token getTokenIfNeedToExecute( clientEnv, outputPermissions, remoteDefaultPlatformProperties, - cachedOutputMetadata)) { + cachedOutputMetadata, + leaseService)) { if (entry != null) { removeCacheEntry(action); } @@ -524,7 +535,8 @@ private boolean mustExecute( Map clientEnv, OutputPermissions outputPermissions, Map remoteDefaultPlatformProperties, - @Nullable CachedOutputMetadata cachedOutputMetadata) + @Nullable CachedOutputMetadata cachedOutputMetadata, + @Nullable LeaseService leaseService) throws InterruptedException { // Unconditional execution can be applied only for actions that are allowed to be executed. if (unconditionalExecution(action)) { @@ -544,7 +556,7 @@ private boolean mustExecute( actionCache.accountMiss(MissReason.CORRUPTED_CACHE_ENTRY); return true; } else if (validateArtifacts( - entry, action, actionInputs, metadataHandler, true, cachedOutputMetadata)) { + entry, action, actionInputs, metadataHandler, true, cachedOutputMetadata, leaseService)) { reportChanged(handler, action); actionCache.accountMiss(MissReason.DIFFERENT_FILES); return true; @@ -747,7 +759,8 @@ private void checkMiddlemanAction( action.getInputs(), metadataHandler, false, - /*cachedOutputMetadata=*/ null)) { + /*cachedOutputMetadata=*/ null, + /*leaseService=*/ null)) { reportChanged(handler, action); actionCache.accountMiss(MissReason.DIFFERENT_FILES); changed = true; @@ -788,7 +801,8 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit( MetadataHandler metadataHandler, ArtifactExpander artifactExpander, Map remoteDefaultPlatformProperties, - boolean loadCachedOutputMetadata) + boolean loadCachedOutputMetadata, + @Nullable LeaseService leaseService) throws InterruptedException { if (action != null) { removeCacheEntry(action); @@ -802,7 +816,8 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit( metadataHandler, artifactExpander, remoteDefaultPlatformProperties, - loadCachedOutputMetadata); + loadCachedOutputMetadata, + leaseService); } /** Returns an action key. It is always set to the first output exec path string. */ diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index 937b3538e20afc..fad8bbc3834712 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java @@ -503,6 +503,8 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source))); } + // TODO(chiwang): setting expiredAtEpochMilli to 0 for now. Save/Load it once the implementation + // details in other part are clear. return RemoteFileArtifactValue.create( digest, size, locationIndex, actionId, materializationExecPath); } 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 b192c95f55e008..8fa81940ba2a16 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -112,6 +112,7 @@ java_library( "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:failure_details_java_proto", + "//src/main/protobuf:remote_lease_java_proto", "//src/main/protobuf:spawn_java_proto", "//third_party:auth", "//third_party:caffeine", diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index 8eab0e6666479f..f458139daf3387 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.buildtool.buildevent.ProfilerStartedEvent; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.CachePolicy; import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode; @@ -253,7 +254,9 @@ private Single> queryRemoteCache( if (digestsToQuery.isEmpty()) { return Single.just(knownRemotePaths); } - return toSingle(() -> remoteCache.findMissingDigests(context, digestsToQuery), executor) + return toSingle( + () -> remoteCache.findMissingDigests(context, Intention.WRITE, digestsToQuery), + executor) .onErrorResumeNext( error -> { reporterUploadError(error); diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java index c605335a8b81d4..1a925469a692e4 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java @@ -178,7 +178,7 @@ public static boolean isRemoteCacheOptions(RemoteOptions options) { @Override public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { + RemoteActionExecutionContext context, Intention intention, Iterable digests) { if (Iterables.isEmpty(digests)) { return Futures.immediateFuture(ImmutableSet.of()); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 4d59c2d73b75b0..dea0d88f6d0120 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -51,6 +51,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.channels.ReadableByteChannel; +import java.time.Instant; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -74,6 +75,8 @@ public class RemoteActionFileSystem extends DelegateFileSystem { private final ActionInputMap inputArtifactData; private final ImmutableMap outputMapping; private final RemoteActionInputFetcher inputFetcher; + private final RemoteLeaseService remoteLeaseService; + private final RemoteInMemoryFileSystem remoteOutputTree; @Nullable private MetadataInjector metadataInjector = null; @@ -84,7 +87,8 @@ public class RemoteActionFileSystem extends DelegateFileSystem { String relativeOutputPath, ActionInputMap inputArtifactData, Iterable outputArtifacts, - RemoteActionInputFetcher inputFetcher) { + RemoteActionInputFetcher inputFetcher, + RemoteLeaseService remoteLeaseService) { super(localDelegate); this.execRoot = checkNotNull(execRootFragment, "execRootFragment"); this.outputBase = execRoot.getRelative(checkNotNull(relativeOutputPath, "relativeOutputPath")); @@ -92,6 +96,8 @@ public class RemoteActionFileSystem extends DelegateFileSystem { this.outputMapping = stream(outputArtifacts).collect(toImmutableMap(Artifact::getExecPath, a -> a)); this.inputFetcher = checkNotNull(inputFetcher, "inputFetcher"); + this.remoteLeaseService = remoteLeaseService; + this.remoteOutputTree = new RemoteInMemoryFileSystem(getDigestFunction()); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index 43f9994623069a..11dea89ea5350c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.exec.SpawnProgressEvent; import com.google.devtools.build.lib.remote.common.LazyFileOutputStream; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException; import com.google.devtools.build.lib.remote.common.ProgressStatusListener; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; @@ -115,11 +116,11 @@ public CachedActionResult downloadActionResult( * guaranteed to be a subset of {@code digests}. */ public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { + RemoteActionExecutionContext context, Intention intention, Iterable digests) { if (Iterables.isEmpty(digests)) { return immediateFuture(ImmutableSet.of()); } - return cacheProtocol.findMissingDigests(context, digests); + return cacheProtocol.findMissingDigests(context, intention, digests); } /** Returns whether the action cache supports updating action results. */ 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 d614e52aac9881..b5232a3bc11297 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 @@ -33,6 +33,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.merkletree.MerkleTree; @@ -252,7 +253,7 @@ private Single> findMissingBlobs( if (digestsToQuery.isEmpty()) { return immediateFuture(ImmutableSet.of()); } - return findMissingDigests(context, digestsToQuery); + return findMissingDigests(context, Intention.WRITE, digestsToQuery); }, directExecutor()) .map( 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 b73fbc85216eb1..25f5e73ac1bfb1 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 @@ -788,8 +788,7 @@ private void createSymlinks(Iterable symlinks) throws IOExcepti } } - private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata metadata) - throws IOException { + private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata metadata) throws IOException { FileSystem actionFileSystem = action.getSpawnExecutionContext().getActionFileSystem(); checkState(actionFileSystem instanceof RemoteActionFileSystem); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteLeaseService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteLeaseService.java new file mode 100644 index 00000000000000..873f4dee934058 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteLeaseService.java @@ -0,0 +1,342 @@ +package com.google.devtools.build.lib.remote; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + +import build.bazel.remote.execution.v2.Digest; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.remote.RemoteLease.LeaseStore; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.util.DigestUtil; +import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; +import com.google.devtools.build.lib.remote.util.Utils; +import com.google.devtools.build.lib.vfs.LeaseService; +import com.google.devtools.build.lib.vfs.Path; +import com.google.protobuf.ByteString; +import java.io.IOException; +import java.time.Duration; +import java.time.Instant; +import java.util.Arrays; +import java.util.Collection; +import java.util.TreeMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.locks.ReentrantLock; +import javax.annotation.concurrent.GuardedBy; + +public class RemoteLeaseService implements LeaseService { + private final String buildRequestId; + private final String commandId; + private final boolean verboseFailures; + private final Path cacheDirectory; + private final EventHandler eventHandler; + private final RemoteCache remoteCache; + private final Duration remoteCacheAge; + private final Duration remoteCacheRenewInterval; + + private volatile boolean shouldRenew = false; + private Thread renewThread; + + private final ReentrantLock leaseLock = new ReentrantLock(); + + // Use Map instead of Set because we need to get the reference to the Lease and check + // `expireAtEpochMilli` when checking whether it is alive. + @GuardedBy("leaseLock") + private final TreeMap leases = + new TreeMap<>( + (o1, o2) -> { + if (o1.equals(o2)) { + return 0; + } + return o1.expireAtEpochMilli < o2.expireAtEpochMilli ? -1 : 1; + }); + + @VisibleForTesting + static class Lease { + private static final Lease MIN = now(0); + + // This doesn't contribute to the equality of the lease but is used for sorting so we can + // qucikly collect a set of leases that are expired. + private final long expireAtEpochMilli; + private final byte[] digest; + private final long size; + private final int locationIndex; + + private static Lease now(long now) { + return new Lease(now, new byte[0], /* size= */ 0, /* locationIndex= */ 0); + } + + static Lease create(long expireAtEpochMilli, RemoteFileArtifactValue metadata) { + return new Lease( + expireAtEpochMilli, + metadata.getDigest(), + metadata.getSize(), + metadata.getLocationIndex()); + } + + @VisibleForTesting + Lease(long expireAtEpochMilli, byte[] digest, long size, int locationIndex) { + this.expireAtEpochMilli = expireAtEpochMilli; + this.digest = digest; + this.size = size; + this.locationIndex = locationIndex; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof Lease)) { + return false; + } + Lease lease = (Lease) o; + return size == lease.size + && locationIndex == lease.locationIndex + && Arrays.equals(digest, lease.digest); + } + + @Override + public int hashCode() { + return Objects.hashCode(Arrays.hashCode(digest), size, locationIndex); + } + } + + public RemoteLeaseService( + String buildRequestId, + String commandId, + boolean verboseFailures, + Path cacheDirectory, + EventHandler eventHandler, + RemoteCache remoteCache, + Duration remoteCacheAge, + Duration remoteCacheRenewInterval) { + this.buildRequestId = buildRequestId; + this.commandId = commandId; + this.verboseFailures = verboseFailures; + this.cacheDirectory = cacheDirectory; + this.eventHandler = eventHandler; + this.remoteCache = remoteCache; + this.remoteCacheAge = remoteCacheAge; + this.remoteCacheRenewInterval = remoteCacheRenewInterval; + } + + public void startBuild() { + checkState(renewThread == null, "renewThread should be null"); + + readLeases(); + + try { + var now = Instant.now().toEpochMilli(); + renewLeases(now, getLeasesToRenew(now)); + } catch (InterruptedException | ExecutionException e) { + eventHandler.handle(Event.warn(Utils.grpcAwareErrorMessage(e, verboseFailures))); + } + + shouldRenew = true; + renewThread = new Thread(this::renewThreadMain, "remote-lease-renew"); + renewThread.setDaemon(true); + renewThread.start(); + } + + public void finalizeBuild() { + checkState(renewThread != null, "renewThread shouldn't be null"); + + shouldRenew = false; + renewThread.interrupt(); + renewThread = null; + + writeLeases(); + } + + private void readLeases() { + Path leasesPath = getLeasesPath(); + if (!leasesPath.exists()) { + return; + } + + try (var silentClosable = Profiler.instance().profile("read leases")) { + var leaseStore = LeaseStore.parseFrom(leasesPath.getInputStream()); + leaseLock.lock(); + try { + for (var persistedLease : leaseStore.getLeasesList()) { + var lease = + new Lease( + persistedLease.getExpireAtEpochMilli(), + persistedLease.getDigest().toByteArray(), + persistedLease.getSize(), + persistedLease.getLocationIndex()); + this.leases.put(lease, lease); + } + } finally { + leaseLock.unlock(); + } + } catch (IOException e) { + eventHandler.handle(Event.warn(Utils.grpcAwareErrorMessage(e, verboseFailures))); + } + } + + private void writeLeases() { + try (var silentClosable = Profiler.instance().profile("write leases")) { + var leaseStore = LeaseStore.newBuilder(); + + leaseLock.lock(); + try { + for (var lease : this.leases.keySet()) { + leaseStore.addLeases( + RemoteLease.Lease.newBuilder() + .setExpireAtEpochMilli(lease.expireAtEpochMilli) + .setDigest(ByteString.copyFrom(lease.digest)) + .setSize(lease.size) + .setLocationIndex(lease.locationIndex) + .build()); + } + } finally { + leaseLock.unlock(); + } + + Path leasesPath = getLeasesPath(); + try { + checkNotNull(leasesPath.getParentDirectory()).createDirectoryAndParents(); + leaseStore.build().writeTo(leasesPath.getOutputStream()); + } catch (IOException e) { + eventHandler.handle(Event.warn(Utils.grpcAwareErrorMessage(e, verboseFailures))); + } + } + } + + private Path getLeasesPath() { + return cacheDirectory.getRelative("lease_store"); + } + + public void add(RemoteFileArtifactValue metadata) { + var now = Instant.now().toEpochMilli(); + add(Lease.create(now + remoteCacheAge.toMillis(), metadata)); + } + + @VisibleForTesting + void add(Lease lease) { + leaseLock.lock(); + try { + leases.put(lease, lease); + } finally { + leaseLock.unlock(); + } + } + + @VisibleForTesting + boolean isAlive(Lease lease) { + return isAlive(lease.digest, lease.size, lease.locationIndex); + } + + @Override + public boolean isAlive(byte[] digest, long size, int locationIndex) { + var now = Instant.now().toEpochMilli(); + leaseLock.lock(); + try { + var lease = leases.get(new Lease(/* expireAtEpochMilli */ 0, digest, size, locationIndex)); + if (lease == null) { + return false; + } + return now < lease.expireAtEpochMilli; + } finally { + leaseLock.unlock(); + } + } + + @VisibleForTesting + void renewLeases(long now, Collection leasesToRenew) + throws InterruptedException, ExecutionException { + if (leasesToRenew.isEmpty()) { + return; + } + + try (var silentCloseable = Profiler.instance().profile("renew leases")) { + ImmutableSet renewedLeases = doRenewLease(now, leasesToRenew); + + leaseLock.lock(); + try { + for (var lease : renewedLeases) { + if (lease.expireAtEpochMilli <= now) { + leases.remove(lease); + } else { + leases.put(lease, lease); + } + } + } finally { + leaseLock.unlock(); + } + } + } + + private ImmutableSet doRenewLease(long now, Collection leases) + throws InterruptedException, ExecutionException { + var metadata = + TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "remote-lease-renew", null); + var context = RemoteActionExecutionContext.create(metadata); + + Iterable digestsToQuery = + () -> + leases.stream() + .map(lease -> DigestUtil.buildDigest(lease.digest, lease.size)) + .iterator(); + // TODO: disk cache, combined cache? + ImmutableSet missingDigests = + remoteCache.findMissingDigests(context, Intention.READ, digestsToQuery).get(); + + var result = ImmutableSet.builderWithExpectedSize(leases.size()); + for (var lease : leases) { + long expireAtEpochMilli = 0; + if (!missingDigests.contains(DigestUtil.buildDigest(lease.digest, lease.size))) { + expireAtEpochMilli = now + remoteCacheAge.toMillis(); + } + result.add(new Lease(expireAtEpochMilli, lease.digest, lease.size, lease.locationIndex)); + } + return result.build(); + } + + private ImmutableSet getLeasesToRenew(long now) { + leaseLock.lock(); + try { + return ImmutableSet.copyOf(leases.subMap(Lease.MIN, Lease.now(now)).keySet()); + } finally { + leaseLock.unlock(); + } + } + + private void renewThreadMain() { + while (shouldRenew) { + try { + Thread.sleep(remoteCacheRenewInterval.toMillis()); + + var now = Instant.now().toEpochMilli(); + renewLeases(now, getLeasesToRenew(now)); + } catch (ExecutionException e) { + eventHandler.handle(Event.warn(Utils.grpcAwareErrorMessage(e, verboseFailures))); + } catch (InterruptedException e) { + return; + } + } + } + + @VisibleForTesting + ImmutableSet getAllLeases() { + leaseLock.lock(); + try { + return ImmutableSet.copyOf(leases.keySet()); + } finally { + leaseLock.unlock(); + } + } + + @VisibleForTesting + RemoteCache getRemoteCache() { + return remoteCache; + } +} 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 6f181b8f847e7a..1fd670b3b34c26 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 @@ -52,6 +52,7 @@ import com.google.devtools.build.lib.buildeventstream.LocalFilesArtifactUploader; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.buildtool.BuildRequestOptions; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -131,6 +132,7 @@ public final class RemoteModule extends BlazeModule { @Nullable private ExecutorService executorService; @Nullable private RemoteActionContextProvider actionContextProvider; @Nullable private RemoteActionInputFetcher actionInputFetcher; + @Nullable private RemoteLeaseService remoteLeaseService; @Nullable private ToplevelArtifactsDownloader toplevelArtifactsDownloader; @Nullable private RemoteOptions remoteOptions; @Nullable private RemoteOutputService remoteOutputService; @@ -256,6 +258,7 @@ public void workspaceInit( public void beforeCommand(CommandEnvironment env) throws AbruptExitException { Preconditions.checkState(actionContextProvider == null, "actionContextProvider must be null"); Preconditions.checkState(actionInputFetcher == null, "actionInputFetcher must be null"); + Preconditions.checkState(remoteLeaseService == null, "remoteLeaseService must be null"); Preconditions.checkState(remoteOptions == null, "remoteOptions must be null"); Preconditions.checkState(tempPathGenerator == null, "tempPathGenerator must be null"); Preconditions.checkState(patternsToDownload == null, "patternsToDownload must be null"); @@ -830,6 +833,7 @@ public void afterCommand() throws AbruptExitException { remoteDownloaderSupplier.set(null); actionContextProvider = null; actionInputFetcher = null; + remoteLeaseService = null; toplevelArtifactsDownloader = null; remoteOptions = null; remoteOutputService = null; @@ -975,6 +979,22 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB return null; }); env.getEventBus().register(toplevelArtifactsDownloader); + + boolean verboseFailures = false; + ExecutionOptions executionOptions = env.getOptions().getOptions(ExecutionOptions.class); + if (executionOptions != null) { + verboseFailures = executionOptions.verboseFailures; + } + remoteLeaseService = new RemoteLeaseService( + env.getBuildRequestId(), + env.getCommandId().toString(), + verboseFailures, + env.getBlazeWorkspace().getCacheDirectory(), + env.getReporter(), + actionContextProvider.getRemoteCache(), + remoteOptions.remoteCacheAge, + remoteOptions.remoteCacheRenewInternal); + remoteOutputService.setRemoteLeaseService(remoteLeaseService); } } 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 a293b40408282f..f218ced5764e02 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 @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.ActionInputMap; 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.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.actions.cache.MetadataInjector; @@ -29,6 +30,7 @@ import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.BatchStat; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.LeaseService; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.PathFragment; @@ -39,15 +41,24 @@ import java.util.UUID; import javax.annotation.Nullable; -/** Output service implementation for the remote module */ +/** + * Output service implementation for the remote module + */ public class RemoteOutputService implements OutputService { - @Nullable private RemoteActionInputFetcher actionInputFetcher; + @Nullable + private RemoteActionInputFetcher actionInputFetcher; + @Nullable + private RemoteLeaseService remoteLeaseService; void setActionInputFetcher(RemoteActionInputFetcher actionInputFetcher) { this.actionInputFetcher = Preconditions.checkNotNull(actionInputFetcher, "actionInputFetcher"); } + void setRemoteLeaseService(RemoteLeaseService remoteLeaseService) { + this.remoteLeaseService = remoteLeaseService; + } + @Override public ActionFileSystemType actionFileSystemType() { return actionInputFetcher != null @@ -66,13 +77,15 @@ public FileSystem createActionFileSystem( Iterable outputArtifacts, boolean rewindingEnabled) { Preconditions.checkNotNull(actionInputFetcher, "actionInputFetcher"); + Preconditions.checkNotNull(remoteLeaseService, "remoteLeaseService"); return new RemoteActionFileSystem( delegateFileSystem, execRootFragment, relativeOutputPath, inputArtifactData, outputArtifacts, - actionInputFetcher); + actionInputFetcher, + remoteLeaseService); } @Override @@ -92,6 +105,9 @@ public String getFilesSystemName() { @Override public ModifiedFileSet startBuild( EventHandler eventHandler, UUID buildId, boolean finalizeActions) throws AbruptExitException { + if (remoteLeaseService != null) { + remoteLeaseService.startBuild(); + } return ModifiedFileSet.EVERYTHING_MODIFIED; } @@ -104,7 +120,9 @@ public void flushOutputTree() throws InterruptedException { @Override public void finalizeBuild(boolean buildSuccessful) { - // Intentionally left empty. + if (remoteLeaseService != null) { + remoteLeaseService.finalizeBuild(); + } } @Override @@ -113,10 +131,28 @@ public void flushActionFileSystem(FileSystem actionFileSystem) throws IOExceptio } @Override - public void finalizeAction(Action action, MetadataHandler metadataHandler) { + public void finalizeAction(Action action, MetadataHandler metadataHandler) throws IOException { if (actionInputFetcher != null) { actionInputFetcher.finalizeAction(action, metadataHandler); } + + if (remoteLeaseService != null) { + for (var output : action.getOutputs()) { + if (output.isTreeArtifact()) { + var metadata = metadataHandler.getTreeArtifactValue((Artifact.SpecialArtifact) output); + for (var treeFileMetadata : metadata.getChildValues().values()) { + if (treeFileMetadata.isRemote()) { + remoteLeaseService.add((FileArtifactValue.RemoteFileArtifactValue) treeFileMetadata); + } + } + } else { + var metadata = metadataHandler.getMetadata(output); + if (metadata != null && metadata.isRemote()) { + remoteLeaseService.add((FileArtifactValue.RemoteFileArtifactValue) metadata); + } + } + } + } } @Nullable @@ -163,7 +199,14 @@ public ArtifactPathResolver createPathResolverForArtifactValues( relativeOutputPath, actionInputMap, ImmutableList.of(), - actionInputFetcher); + actionInputFetcher, + remoteLeaseService); return ArtifactPathResolver.createPathResolver(remoteFileSystem, fileSystem.getPath(execRoot)); } + + @Nullable + @Override + public LeaseService getLeaseService() { + return remoteLeaseService; + } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java index 07c02ac35688db..f48d698240db28 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java +++ b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; @@ -488,7 +489,7 @@ public Single uploadAsync( String outputPrefix = "cas/"; Flowable bulkTransfers = - toSingle(() -> remoteCache.findMissingDigests(context, digests), directExecutor()) + toSingle(() -> remoteCache.findMissingDigests(context, Intention.WRITE, digests), directExecutor()) .doOnSubscribe(d -> reportUploadStarted(reporter, action, outputPrefix, digests)) .doOnError(error -> reportUploadFinished(reporter, action, outputPrefix, digests)) .doOnDispose(() -> reportUploadFinished(reporter, action, outputPrefix, digests)) diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java b/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java index f682e93ca4d197..c5cc9dad4eaa67 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java @@ -20,6 +20,11 @@ /** Supports querying a remote cache whether it contains a list of blobs. */ public interface MissingDigestsFinder { + enum Intention { + WRITE, + READ; + } + /** * Returns a set of digests that the remote cache does not know about. The returned set is * guaranteed to be a subset of {@code digests}. @@ -27,5 +32,5 @@ public interface MissingDigestsFinder { * @param digests The list of digests to look for. */ ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests); + RemoteActionExecutionContext context, Intention intention, Iterable digests); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java index 37ccd2cd4b456a..ca5da8e3b09d7f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java @@ -21,6 +21,7 @@ import build.bazel.remote.execution.v2.Digest; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.remote.common.LazyFileOutputStream; @@ -108,15 +109,48 @@ public ListenableFuture uploadBlob( @Override public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Intention intention, Iterable digests) { + switch (intention) { + case READ: + return findMissingDigestsForRead(context, digests); + case WRITE: + return findMissingDigestsForWrite(context, digests); + } + + throw new IllegalStateException("unreachable"); + } + + private ListenableFuture> findMissingDigestsForRead( + RemoteActionExecutionContext context, Iterable digests) { + if (context.getReadCachePolicy().allowRemoteCache() + && context.getReadCachePolicy().allowDiskCache()) { + ListenableFuture> remoteQuery = + remoteCache.findMissingDigests(context, Intention.READ, digests); + ListenableFuture> diskQuery = + diskCache.findMissingDigests(context, Intention.READ, digests); + return Futures.whenAllSucceed(remoteQuery, diskQuery) + .call( + () -> ImmutableSet.copyOf(Sets.intersection(remoteQuery.get(), diskQuery.get())), + directExecutor()); + } else if (context.getReadCachePolicy().allowRemoteCache()) { + return remoteCache.findMissingDigests(context, Intention.READ, digests); + } else if (context.getReadCachePolicy().allowDiskCache()) { + return diskCache.findMissingDigests(context, Intention.READ, digests); + } else { + return immediateFuture(ImmutableSet.copyOf(digests)); + } + } + + private ListenableFuture> findMissingDigestsForWrite( RemoteActionExecutionContext context, Iterable digests) { ListenableFuture> diskQuery = immediateFuture(ImmutableSet.of()); if (context.getWriteCachePolicy().allowDiskCache()) { - diskQuery = diskCache.findMissingDigests(context, digests); + diskQuery = diskCache.findMissingDigests(context, Intention.WRITE, digests); } ListenableFuture> remoteQuery = immediateFuture(ImmutableSet.of()); if (context.getWriteCachePolicy().allowRemoteCache()) { - remoteQuery = remoteCache.findMissingDigests(context, digests); + remoteQuery = remoteCache.findMissingDigests(context, Intention.WRITE, digests); } ListenableFuture> diskQueryFinal = diskQuery; diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java index 55d6b43581f8dd..a89c9464671e8b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java @@ -35,6 +35,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.HashSet; import java.util.UUID; import javax.annotation.Nullable; @@ -206,10 +207,17 @@ public ListenableFuture uploadBlob( @Override public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { - // Both upload and download check if the file exists before doing I/O. So we don't - // have to do it here. - return Futures.immediateFuture(ImmutableSet.copyOf(digests)); + RemoteActionExecutionContext context, Intention intention, Iterable digests) { + var result = new HashSet(); + for (var digest : digests) { + if (digest.getSizeBytes() == 0) { + continue; + } + if (!toPath(digest.getHash(), /* actionResult= */ false).exists()) { + result.add(digest); + } + } + return Futures.immediateFuture(ImmutableSet.copyOf(result)); } protected Path toPathNoSplit(String key) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java index 6c6d4cc6f897fc..6aef7f50ce2d83 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java @@ -688,7 +688,10 @@ public ListenableFuture uploadBlob( @Override public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { + RemoteActionExecutionContext context, Intention intention, Iterable digests) { + if (intention == Intention.READ) { + throw new UnsupportedOperationException("HttpCache doesn't support FindMissingBlobs."); + } return Futures.immediateFuture(ImmutableSet.copyOf(digests)); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/CommonRemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/CommonRemoteOptions.java index 27da47a8fb2da0..b1d0912ac064ba 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/CommonRemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/CommonRemoteOptions.java @@ -13,11 +13,16 @@ // limitations under the License. package com.google.devtools.build.lib.remote.options; +import com.google.devtools.common.options.Converter; +import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionsBase; +import com.google.devtools.common.options.OptionsParsingException; +import java.time.Duration; import java.util.List; +import java.util.regex.Pattern; /** Options for remote execution and distributed caching that shared between Bazel and Blaze. */ public class CommonRemoteOptions extends OptionsBase { @@ -33,4 +38,49 @@ public class CommonRemoteOptions extends OptionsBase { + " the client to request certain artifacts that might be needed locally (e.g. IDE" + " support)") public List remoteDownloadRegex; + + @Option( + name = "experimental_remote_cache_age", + defaultValue = "2h", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.EXECUTION}, + converter = RemoteDurationConverter.class, + help = "The guaranteed minimal age of blobs in the remote cache after their digests are " + + "recently referenced e.g. by an ActionResult. Bazel does several optimizations based on " + + "the blobs' age e.g. doesn't repeatedly call GetActionResult in an incremental build. " + + "The value should be set slightly less than the real age since there is a gap between " + + "when the server returns the digests and when Bazel receives them." + ) + public Duration remoteCacheAge; + + @Option( + name = "experimental_remote_cache_renew_interval", + defaultValue = "2h", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.EXECUTION}, + converter = RemoteDurationConverter.class, + help = "TODO" + ) + public Duration remoteCacheRenewInternal; + + /** + * Returns the specified duration. Assumes seconds if unitless. + */ + public static class RemoteDurationConverter extends Converter.Contextless { + + private static final Pattern UNITLESS_REGEX = Pattern.compile("^[0-9]+$"); + + @Override + public Duration convert(String input) throws OptionsParsingException { + if (UNITLESS_REGEX.matcher(input).matches()) { + input += "s"; + } + return new Converters.DurationConverter().convert(input, /*conversionContext=*/ null); + } + + @Override + public String getTypeDescription() { + return "An immutable length of time."; + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index c812a423aa3f77..53bdc3af3ab760 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code; import com.google.devtools.build.lib.util.OptionsUtils; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Converters.AssignmentConverter; import com.google.devtools.common.options.EnumConverter; @@ -32,7 +31,6 @@ import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionMetadataTag; -import com.google.devtools.common.options.OptionsParsingException; import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; import java.time.Duration; @@ -40,7 +38,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.SortedMap; -import java.util.regex.Pattern; /** Options for remote execution and distributed caching for Bazel only. */ public final class RemoteOptions extends CommonRemoteOptions { @@ -203,7 +200,7 @@ public final class RemoteOptions extends CommonRemoteOptions { defaultValue = "60s", documentationCategory = OptionDocumentationCategory.REMOTE, effectTags = {OptionEffectTag.UNKNOWN}, - converter = RemoteTimeoutConverter.class, + converter = RemoteDurationConverter.class, help = "The maximum amount of time to wait for remote execution and cache calls. For the REST" + " cache, this is both the connect and the read timeout. Following units can be" @@ -224,24 +221,6 @@ public final class RemoteOptions extends CommonRemoteOptions { + "When not set, it will default to \"${hostname}/${instance_name}\".") public String remoteBytestreamUriPrefix; - /** Returns the specified duration. Assumes seconds if unitless. */ - public static class RemoteTimeoutConverter extends Converter.Contextless { - private static final Pattern UNITLESS_REGEX = Pattern.compile("^[0-9]+$"); - - @Override - public Duration convert(String input) throws OptionsParsingException { - if (UNITLESS_REGEX.matcher(input).matches()) { - input += "s"; - } - return new Converters.DurationConverter().convert(input, /*conversionContext=*/ null); - } - - @Override - public String getTypeDescription() { - return "An immutable length of time."; - } - } - @Option( name = "remote_accept_cached", defaultValue = "true", diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java index 95563cff258630..ed866fa12730f9 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java @@ -167,7 +167,7 @@ public Path getInstallBase() { * run concurrent instances of blaze in different clients without attempting to concurrently write * to the same action cache on disk, which might not be safe. */ - private Path getCacheDirectory() { + public Path getCacheDirectory() { return getOutputBase().getChild("action_cache"); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java index 50ab2f99b171f8..0e07e807102340 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.FileStateType; import com.google.devtools.build.lib.concurrent.ExecutorUtil; import com.google.devtools.build.lib.concurrent.Sharder; @@ -45,6 +46,7 @@ import com.google.devtools.build.lib.vfs.BatchStat; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileStatusWithDigest; +import com.google.devtools.build.lib.vfs.LeaseService; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -58,6 +60,7 @@ import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.WalkableGraph; import java.io.IOException; +import java.time.Instant; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -177,7 +180,8 @@ Collection getDirtyActionValues( @Nullable final BatchStat batchStatter, ModifiedFileSet modifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) + ModifiedOutputsReceiver modifiedOutputsReceiver, + @Nullable LeaseService leaseService) throws InterruptedException { if (modifiedOutputFiles == ModifiedFileSet.NOTHING_MODIFIED) { logger.atInfo().log("Not checking for dirty actions since nothing was modified"); @@ -239,7 +243,8 @@ public NavigableSet get() { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver) + modifiedOutputsReceiver, + leaseService) : batchStatJob( dirtyKeys, shard, @@ -247,7 +252,8 @@ public NavigableSet get() { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver); + modifiedOutputsReceiver, + leaseService); executor.execute(job); } @@ -273,7 +279,8 @@ private Runnable batchStatJob( ImmutableSet knownModifiedOutputFiles, Supplier> sortedKnownModifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + @Nullable LeaseService leaseService) { return () -> { Map> fileToKeyAndValue = new HashMap<>(); Map> treeArtifactsToKeyAndValue = @@ -331,7 +338,8 @@ && shouldCheckFile(knownModifiedOutputFiles, artifact)) { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver) + modifiedOutputsReceiver, + leaseService) .run(); return; } catch (InterruptedException e) { @@ -393,7 +401,8 @@ private Runnable outputStatJob( ImmutableSet knownModifiedOutputFiles, Supplier> sortedKnownModifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + @Nullable LeaseService leaseService) { return new Runnable() { @Override public void run() { @@ -405,7 +414,8 @@ public void run() { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver)) { + modifiedOutputsReceiver, + leaseService)) { dirtyKeys.add(keyAndValue.getFirst()); } } @@ -441,7 +451,8 @@ private boolean artifactIsDirtyWithDirectSystemCalls( ImmutableSet knownModifiedOutputFiles, boolean trustRemoteArtifacts, Map.Entry entry, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + @Nullable LeaseService leaseService) { Artifact file = entry.getKey(); FileArtifactValue lastKnownData = entry.getValue(); if (file.isMiddlemanArtifact() || !shouldCheckFile(knownModifiedOutputFiles, file)) { @@ -454,6 +465,12 @@ private boolean artifactIsDirtyWithDirectSystemCalls( fileMetadata.getType() == FileStateType.NONEXISTENT && lastKnownData.isRemote() && trustRemoteArtifacts; + if (leaseService != null && trustRemoteValue) { + if (!leaseService.isAlive( + lastKnownData.getDigest(), lastKnownData.getSize(), lastKnownData.getLocationIndex())) { + trustRemoteValue = false; + } + } if (!trustRemoteValue && fileMetadata.couldBeModifiedSince(lastKnownData)) { modifiedOutputsReceiver.reportModifiedOutputFile( fileMetadata.getType() != FileStateType.NONEXISTENT @@ -475,11 +492,16 @@ private boolean actionValueIsDirtyWithDirectSystemCalls( ImmutableSet knownModifiedOutputFiles, Supplier> sortedKnownModifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + @Nullable LeaseService leaseService) { boolean isDirty = false; for (Map.Entry entry : actionValue.getAllFileValues().entrySet()) { if (artifactIsDirtyWithDirectSystemCalls( - knownModifiedOutputFiles, trustRemoteArtifacts, entry, modifiedOutputsReceiver)) { + knownModifiedOutputFiles, + trustRemoteArtifacts, + entry, + modifiedOutputsReceiver, + leaseService)) { isDirty = true; } } @@ -495,7 +517,8 @@ private boolean actionValueIsDirtyWithDirectSystemCalls( knownModifiedOutputFiles, trustRemoteArtifacts, childEntry, - modifiedOutputsReceiver)) { + modifiedOutputsReceiver, + leaseService)) { isDirty = true; } } @@ -510,7 +533,8 @@ private boolean actionValueIsDirtyWithDirectSystemCalls( Maps.immutableEntry( archivedRepresentation.archivedTreeFileArtifact(), archivedRepresentation.archivedFileValue()), - modifiedOutputsReceiver)) + modifiedOutputsReceiver, + leaseService)) .orElse(false); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 9e692989728ca7..e481bfe461f7b6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -84,6 +84,7 @@ import com.google.devtools.build.lib.vfs.BatchStat; import com.google.devtools.build.lib.vfs.FileStateKey; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.LeaseService; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; @@ -788,6 +789,7 @@ public void detectModifiedOutputFiles( new FilesystemValueChecker( Preconditions.checkNotNull(tsgm.get()), syscallCache, fsvcThreads); BatchStat batchStatter = outputService == null ? null : outputService.getBatchStatter(); + LeaseService leaseService = outputService == null ? null : outputService.getLeaseService(); recordingDiffer.invalidate( fsvc.getDirtyActionValues( memoizingEvaluator.getValues(), @@ -804,7 +806,8 @@ public void detectModifiedOutputFiles( if (dirtyOutputsCount <= MODIFIED_OUTPUT_PATHS_SAMPLE_SIZE) { outputDirtyFilesExecPathSample.offer(artifact.getExecPathString()); } - })); + }, + leaseService)); logger.atInfo().log("Found %d modified files from last build", modifiedFiles.get()); long stopTime = System.nanoTime(); Profiler.instance() diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index ddf68a280dae4b..cc700b4a5f2f23 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -99,6 +99,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystem.NotASymlinkException; import com.google.devtools.build.lib.vfs.OutputPermissions; +import com.google.devtools.build.lib.vfs.LeaseService; import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType; import com.google.devtools.build.lib.vfs.Path; @@ -601,22 +602,23 @@ Token checkActionCache( RemoteOptions remoteOptions; SortedMap remoteDefaultProperties; EventHandler handler; - boolean loadCachedOutputMetadata; - if (cacheHitSemaphore != null) { try (SilentCloseable c = profiler.profile(ProfilerTask.ACTION_CHECK, "acquiring semaphore")) { cacheHitSemaphore.acquire(); } } + boolean loadCachedOutputMetadata = false; + LeaseService leaseService = null; try (SilentCloseable c = profiler.profile(ProfilerTask.ACTION_CHECK, action.describe())) { remoteOptions = this.options.getOptions(RemoteOptions.class); remoteDefaultProperties = remoteOptions != null ? remoteOptions.getRemoteDefaultExecProperties() : ImmutableSortedMap.of(); - loadCachedOutputMetadata = - outputService - != null; // Only load cached output metadata if remote output service is available + if (outputService != null) { + loadCachedOutputMetadata = true; // Only load cached output metadata if remote output service is available + leaseService = outputService.getLeaseService(); + } handler = options.getOptions(BuildRequestOptions.class).explanationPath != null ? reporter : null; token = @@ -629,7 +631,8 @@ Token checkActionCache( metadataHandler, artifactExpander, remoteDefaultProperties, - loadCachedOutputMetadata); + loadCachedOutputMetadata, + leaseService); if (token == null) { boolean eventPosted = false; @@ -671,7 +674,8 @@ public T getContext(Class type) { metadataHandler, artifactExpander, remoteDefaultProperties, - loadCachedOutputMetadata); + loadCachedOutputMetadata, + leaseService); } } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/LeaseService.java b/src/main/java/com/google/devtools/build/lib/vfs/LeaseService.java new file mode 100644 index 00000000000000..cfc6c164507c6f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/vfs/LeaseService.java @@ -0,0 +1,5 @@ +package com.google.devtools.build.lib.vfs; + +public interface LeaseService { + boolean isAlive(byte[] digest, long size, int locationIndex); +} diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index 53517ef2d16434..6e81dcc0547aa7 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -236,4 +236,9 @@ default ArtifactPathResolver createPathResolverForArtifactValues( default BulkDeleter bulkDeleter() { return null; } + + @Nullable + default LeaseService getLeaseService() { + return null; + } } diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index 809860f5474424..afadf20ad73f1e 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -220,6 +220,16 @@ java_proto_library( deps = [":remote_execution_log_proto"], ) +proto_library( + name = "remote_lease_proto", + srcs = ["remote_lease.proto"], +) + +java_proto_library( + name = "remote_lease_java_proto", + deps = [":remote_lease_proto"], +) + java_library_srcs( name = "remote_execution_log_java_proto_srcs", deps = [":remote_execution_log_java_proto"], diff --git a/src/main/protobuf/remote_lease.proto b/src/main/protobuf/remote_lease.proto new file mode 100644 index 00000000000000..82184ebae20814 --- /dev/null +++ b/src/main/protobuf/remote_lease.proto @@ -0,0 +1,30 @@ +// Copyright 2018 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. + +syntax = "proto3"; + +package remote_lease; + +option java_package = "com.google.devtools.build.lib.remote"; + +message Lease { + uint64 expire_at_epoch_milli = 1; + bytes digest = 2; + uint64 size = 3; + uint32 locationIndex = 4; +} + +message LeaseStore { + repeated Lease leases = 1; +} \ No newline at end of file diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java index ee8f3d7c4685df..eda60719f5dbe8 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java @@ -187,7 +187,8 @@ private void runAction( metadataHandler, /* artifactExpander= */ null, platform, - /* loadCachedOutputMetadata= */ true); + /* loadCachedOutputMetadata= */ true, + /* leaseService= */ null); if (token != null) { // Real action execution would happen here. ActionExecutionContext context = mock(ActionExecutionContext.class); @@ -452,7 +453,8 @@ public void testDeletedConstantMetadataOutputCausesReexecution() throws Exceptio new FakeMetadataHandler(), /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), - /* loadCachedOutputMetadata= */ true)) + /* loadCachedOutputMetadata= */ true, + /* leaseService= */ null)) .isNotNull(); } @@ -579,7 +581,8 @@ public void saveOutputMetadata_remoteFileMetadataLoaded() throws Exception { metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), - /* loadCachedOutputMetadata= */ true); + /* loadCachedOutputMetadata= */ true, + /* leaseService= */ null); assertThat(output.getPath().exists()).isFalse(); assertThat(token).isNull(); @@ -609,7 +612,8 @@ public void saveOutputMetadata_remoteOutputUnavailable_remoteFileMetadataNotLoad metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), - /* loadCachedOutputMetadata= */ false); + /* loadCachedOutputMetadata= */ false, + /* leaseService= */ null); assertThat(output.getPath().exists()).isFalse(); assertThat(token).isNotNull(); @@ -784,7 +788,8 @@ public void saveOutputMetadata_emptyTreeMetadata_notSaved() throws Exception { metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), - /* loadCachedOutputMetadata= */ true); + /* loadCachedOutputMetadata= */ true, + /* leaseService= */ null); assertThat(token).isNull(); assertThat(output.getPath().exists()).isFalse(); @@ -883,7 +888,8 @@ public void saveOutputMetadata_treeMetadata_remoteFileMetadataLoaded() throws Ex metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), - /* loadCachedOutputMetadata= */ true); + /* loadCachedOutputMetadata= */ true, + /* leaseService= */ null); TreeArtifactValue expectedMetadata = createTreeMetadata( @@ -1045,7 +1051,8 @@ public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached( metadataHandler, /* artifactExpander= */ null, /* remoteDefaultPlatformProperties= */ ImmutableMap.of(), - /* loadCachedOutputMetadata= */ true); + /* loadCachedOutputMetadata= */ true, + /* leaseService= */ null); assertThat(token).isNull(); assertStatistics(1, new MissDetailsBuilder().set(MissReason.NOT_CACHED, 1).build()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index 15b5e2e715e922..5418825e130f1c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -365,7 +365,8 @@ public void remoteFileShouldNotBeUploaded_actionFs() throws Exception { outputRoot.getRoot().asPath().relativeTo(execRoot).getPathString(), outputs, ImmutableList.of(artifact), - actionInputFetcher); + actionInputFetcher, + mock(RemoteLeaseService.class)); Path remotePath = remoteFs.getPath(artifact.getPath().getPathString()); assertThat(remotePath.getFileSystem()).isEqualTo(remoteFs); LocalFile file = @@ -428,7 +429,7 @@ public void remoteFileShouldNotBeUploaded_findMissingDigests() throws Exception PathConverter pathConverter = artifactUploader.upload(files).get(); // assert - verify(digestQuerier).findMissingDigests(any(), any()); + verify(digestQuerier).findMissingDigests(any(), any(), any()); verify(remoteCache).uploadFile(any(), eq(localDigest), any()); assertThat(pathConverter.apply(remoteFile)).contains(remoteDigest.getHash()); assertThat(pathConverter.apply(localFile)).contains(localDigest.getHash()); @@ -468,9 +469,11 @@ private RemoteCache newRemoteCache( doAnswer( invocationOnMock -> missingDigestsFinder.findMissingDigests( - invocationOnMock.getArgument(0), invocationOnMock.getArgument(1))) + invocationOnMock.getArgument(0), + invocationOnMock.getArgument(1), + invocationOnMock.getArgument(2))) .when(cacheClient) - .findMissingDigests(any(), any()); + .findMissingDigests(any(), any(), any()); return new RemoteCache( CacheCapabilities.getDefaultInstance(), cacheClient, remoteOptions, DIGEST_UTIL); @@ -500,7 +503,7 @@ public StaticMissingDigestsFinder(ImmutableSet knownDigests) { @Override public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { + RemoteActionExecutionContext context, Intention intention, Iterable digests) { ImmutableSet.Builder missingDigests = ImmutableSet.builder(); for (Digest digest : digests) { if (!knownDigests.contains(digest)) { @@ -517,7 +520,7 @@ private static class AllMissingDigestsFinder implements MissingDigestsFinder { @Override public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { + RemoteActionExecutionContext context, Intention intention, Iterable digests) { return Futures.immediateFuture(ImmutableSet.copyOf(digests)); } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index 487fe3404f1545..5ceb947a277d4a 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -82,7 +82,8 @@ protected RemoteActionFileSystem createActionFileSystem( outputRoot.getRoot().asPath().relativeTo(execRoot).getPathString(), inputs, outputs, - inputFetcher); + inputFetcher, + mock(RemoteLeaseService.class)); remoteActionFileSystem.updateContext(metadataInjector); remoteActionFileSystem.createDirectoryAndParents(outputRoot.getRoot().asPath().asFragment()); return remoteActionFileSystem; diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java index df88d694635e66..3bbffabde8b025 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.exec.util.FakeOwner; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.merkletree.MerkleTree; @@ -231,11 +232,17 @@ public void upload_emptyBlobAndFile_doNotPerformUpload() throws Exception { Path file = execRoot.getRelative("file"); getFromFuture(remoteCache.uploadBlob(context, emptyDigest, ByteString.EMPTY)); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableSet.of(emptyDigest)))) + assertThat( + getFromFuture( + remoteCache.findMissingDigests( + context, Intention.WRITE, ImmutableSet.of(emptyDigest)))) .containsExactly(emptyDigest); getFromFuture(remoteCache.uploadFile(context, emptyDigest, file)); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableSet.of(emptyDigest)))) + assertThat( + getFromFuture( + remoteCache.findMissingDigests( + context, Intention.WRITE, ImmutableSet.of(emptyDigest)))) .containsExactly(emptyDigest); } @@ -281,13 +288,17 @@ public void upload_failedUploads_doNotDeduplicate() throws Exception { doAnswer( invocationOnMock -> inMemoryCacheClient.findMissingDigests( - invocationOnMock.getArgument(0), invocationOnMock.getArgument(1))) + invocationOnMock.getArgument(0), + invocationOnMock.getArgument(1), + invocationOnMock.getArgument(2))) .when(remoteCacheClient) - .findMissingDigests(any(), any()); + .findMissingDigests(any(), any(), any()); RemoteCache remoteCache = newRemoteCache(remoteCacheClient); Digest digest = fakeFileCache.createScratchInput(ActionInputHelper.fromPath("file"), "content"); Path file = execRoot.getRelative("file"); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableList.of(digest)))) + assertThat( + getFromFuture( + remoteCache.findMissingDigests(context, Intention.WRITE, ImmutableList.of(digest)))) .containsExactly(digest); Exception thrown = null; @@ -300,7 +311,9 @@ public void upload_failedUploads_doNotDeduplicate() throws Exception { assertThat(thrown).isInstanceOf(IOException.class); getFromFuture(remoteCache.uploadFile(context, digest, file)); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableList.of(digest)))) + assertThat( + getFromFuture( + remoteCache.findMissingDigests(context, Intention.WRITE, ImmutableList.of(digest)))) .isEmpty(); } @@ -384,7 +397,7 @@ public void ensureInputsPresent_interruptedDuringUploadBlobs_cancelInProgressUpl return findMissingDigestsFuture; }) .when(remoteCache) - .findMissingDigests(any(), any()); + .findMissingDigests(any(), any(), any()); Deque> futures = new ConcurrentLinkedDeque<>(); CountDownLatch uploadBlobCalls = new CountDownLatch(2); doAnswer( 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 b8dce08dcd183a..8b694e35aef9bf 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 @@ -84,6 +84,7 @@ import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; import com.google.devtools.build.lib.remote.common.BulkTransferException; +import com.google.devtools.build.lib.remote.common.MissingDigestsFinder.Intention; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; @@ -1336,7 +1337,9 @@ public void uploadOutputs_uploadDirectory_works() throws Exception { assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build()); ImmutableList toQuery = ImmutableList.of(fooDigest, quxDigest, barDigest); - assertThat(getFromFuture(cache.findMissingDigests(remoteActionExecutionContext, toQuery))) + assertThat( + getFromFuture( + cache.findMissingDigests(remoteActionExecutionContext, Intention.WRITE, toQuery))) .isEmpty(); } @@ -1379,7 +1382,7 @@ public void uploadOutputs_uploadEmptyDirectory_works() throws Exception { assertThat( getFromFuture( cache.findMissingDigests( - remoteActionExecutionContext, ImmutableList.of(barDigest)))) + remoteActionExecutionContext, Intention.WRITE, ImmutableList.of(barDigest)))) .isEmpty(); } @@ -1447,7 +1450,9 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception { assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build()); ImmutableList toQuery = ImmutableList.of(wobbleDigest, quxDigest, barDigest); - assertThat(getFromFuture(cache.findMissingDigests(remoteActionExecutionContext, toQuery))) + assertThat( + getFromFuture( + cache.findMissingDigests(remoteActionExecutionContext, Intention.WRITE, toQuery))) .isEmpty(); } @@ -1525,7 +1530,7 @@ public void uploadOutputs_emptyOutputs_doNotPerformUpload() throws Exception { assertThat( getFromFuture( cache.findMissingDigests( - remoteActionExecutionContext, ImmutableSet.of(emptyDigest)))) + remoteActionExecutionContext, Intention.WRITE, ImmutableSet.of(emptyDigest)))) .containsExactly(emptyDigest); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteLeaseServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteLeaseServiceTest.java new file mode 100644 index 00000000000000..c3b7798c2fe363 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteLeaseServiceTest.java @@ -0,0 +1,157 @@ +package com.google.devtools.build.lib.remote; + +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; + +import build.bazel.remote.execution.v2.CacheCapabilities; +import com.google.common.collect.ImmutableSet; +import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.events.EventCollector; +import com.google.devtools.build.lib.remote.RemoteLeaseService.Lease; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.options.RemoteOptions; +import com.google.devtools.build.lib.remote.util.DigestUtil; +import com.google.devtools.build.lib.remote.util.InMemoryCacheClient; +import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.SyscallCache; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import com.google.devtools.common.options.Options; +import com.google.protobuf.ByteString; +import java.time.Instant; +import java.util.concurrent.ExecutionException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link RemoteLeaseService} */ +@RunWith(JUnit4.class) +public class RemoteLeaseServiceTest { + private static final DigestUtil DIGEST_UTIL = + new DigestUtil(SyscallCache.NO_CACHE, DigestHashFunction.SHA256); + + @Test + public void noLease() { + var leaseService = createLeaseService(); + var metadata = createMetadata("content"); + + assertThat( + leaseService.isAlive( + metadata.getDigest(), metadata.getSize(), metadata.getLocationIndex())) + .isFalse(); + } + + @Test + public void leaseIsAlive() { + var leaseService = createLeaseService(); + var metadata = createMetadata("content"); + var now = Instant.now(); + var lease = Lease.create(now.plusSeconds(3600).toEpochMilli(), metadata); + + leaseService.add(lease); + + assertThat(leaseService.isAlive(lease)).isTrue(); + } + + @Test + public void leaseIsExpired() { + var leaseService = createLeaseService(); + var metadata = createMetadata("content"); + var now = Instant.now(); + var lease = Lease.create(now.minusSeconds(3600).toEpochMilli(), metadata); + + leaseService.add(lease); + + assertThat(leaseService.isAlive(lease)).isFalse(); + } + + @Test + public void addsSameDigestDifferentExpiration_overrideExisting() { + var leaseService = createLeaseService(); + var metadata = createMetadata("content"); + var now = Instant.now(); + var oldLease = Lease.create(now.minusSeconds(3600).toEpochMilli(), metadata); + leaseService.add(oldLease); + + var newLease = Lease.create(now.plusSeconds(3600).toEpochMilli(), metadata); + leaseService.add(newLease); + + assertThat(leaseService.isAlive(newLease)).isTrue(); + assertThat(leaseService.getAllLeases()).hasSize(1); + } + + @Test + public void renew_removeLeasesForMissingFiles() throws ExecutionException, InterruptedException { + var leaseService = createLeaseService(); + var metadata = createMetadata("content"); + var now = Instant.now().toEpochMilli(); + var lease = Lease.create(0, metadata); + leaseService.add(lease); + assertThat(leaseService.getAllLeases()).hasSize(1); + + leaseService.renewLeases(now, ImmutableSet.of(lease)); + + assertThat(leaseService.isAlive(lease)).isFalse(); + assertThat(leaseService.getAllLeases()).isEmpty(); + } + + @Test + public void renew_extendsLeases() throws Exception { + var leaseService = createLeaseService(); + var metadata = createFile(leaseService, "content"); + var now = Instant.now().toEpochMilli(); + var lease = Lease.create(0, metadata); + leaseService.add(lease); + + leaseService.renewLeases(now, ImmutableSet.of(lease)); + + assertThat(leaseService.isAlive(lease)).isTrue(); + } + + private RemoteLeaseService createLeaseService() { + var fileSystem = new InMemoryFileSystem(DigestHashFunction.SHA256); + var cacheDirectory = fileSystem.getPath("/tmp/action_cache"); + var eventHandler = new EventCollector(); + var remoteOptions = Options.getDefaults(RemoteOptions.class); + var remoteCache = + new RemoteCache( + CacheCapabilities.newBuilder().build(), + new InMemoryCacheClient(), + remoteOptions, + DIGEST_UTIL); + return new RemoteLeaseService( + "", + "", + false, + cacheDirectory, + eventHandler, + remoteCache, + remoteOptions.remoteCacheAge, + remoteOptions.remoteCacheRenewInternal); + } + + private RemoteFileArtifactValue createMetadata(String content) { + var bytes = content.getBytes(UTF_8); + var digest = DIGEST_UTIL.compute(bytes); + return RemoteFileArtifactValue.create( + HashCode.fromString(digest.getHash()).asBytes(), bytes.length, 0); + } + + private RemoteFileArtifactValue createFile(RemoteLeaseService leaseService, String content) + throws Exception { + var remoteCache = leaseService.getRemoteCache(); + var metadata = createMetadata("content"); + var context = + RemoteActionExecutionContext.create( + TracingMetadataUtils.buildMetadata( + "build-request-id", "command-id", "action-id", /* actionMetadata= */ null)); + remoteCache + .uploadBlob( + context, + DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()), + ByteString.copyFrom("content".getBytes(UTF_8))) + .get(); + return createMetadata(content); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/remote/options/RemoteOptionsTest.java b/src/test/java/com/google/devtools/build/lib/remote/options/RemoteOptionsTest.java index e6425a4889f650..992a69f2c6be09 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/options/RemoteOptionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/options/RemoteOptionsTest.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.UserExecException; +import com.google.devtools.build.lib.remote.options.CommonRemoteOptions.RemoteDurationConverter; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParsingException; import java.time.Duration; @@ -79,7 +80,7 @@ public void testRemoteTimeoutOptionsConverterWithoutUnit() { try { int seconds = 60; Duration convert = - new RemoteOptions.RemoteTimeoutConverter().convert(String.valueOf(seconds)); + new RemoteDurationConverter().convert(String.valueOf(seconds)); assertThat(Duration.ofSeconds(seconds)).isEqualTo(convert); } catch (OptionsParsingException e) { fail(e.getMessage()); @@ -90,7 +91,7 @@ public void testRemoteTimeoutOptionsConverterWithoutUnit() { public void testRemoteTimeoutOptionsConverterWithUnit() { try { int milliseconds = 60; - Duration convert = new RemoteOptions.RemoteTimeoutConverter().convert(milliseconds + "ms"); + Duration convert = new RemoteDurationConverter().convert(milliseconds + "ms"); assertThat(Duration.ofMillis(milliseconds)).isEqualTo(convert); } catch (OptionsParsingException e) { fail(e.getMessage()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java index 1fb0fe969ef8ee..fd2de119a564c6 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java @@ -146,7 +146,7 @@ public ListenableFuture uploadBlob( @Override public ListenableFuture> findMissingDigests( - RemoteActionExecutionContext context, Iterable digests) { + RemoteActionExecutionContext context, Intention intention, Iterable digests) { return executorService.submit( () -> { ImmutableSet.Builder missingBuilder = ImmutableSet.builder(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index 598e504c309221..d77d0a4f7285c5 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -506,7 +506,8 @@ private Collection getDirtyActionValues(ImmutableMap v batchStat.getBatchStat(fs), ModifiedFileSet.EVERYTHING_MODIFIED, /*trustRemoteArtifacts=*/ false, - (ignored, ignored2) -> {}); + (ignored, ignored2) -> {}, + /*leaseService=*/ null); } private TreeFileArtifact createTreeFileArtifactWithContent( @@ -802,7 +803,8 @@ private void pretendBuildTwoArtifacts( batchStatter, ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .isEmpty(); tsgm.waitForTimestampGranularity(OutErr.SYSTEM_OUT_ERR); @@ -818,7 +820,8 @@ private void checkActionDirtiedByFile( batchStatter, ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .containsExactly(actionKey); assertThat( new FilesystemValueChecker(tsgm, SyscallCache.NO_CACHE, FSVC_THREADS_FOR_TEST) @@ -827,7 +830,8 @@ private void checkActionDirtiedByFile( batchStatter, ModifiedFileSet.EVERYTHING_DELETED, /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .containsExactly(actionKey); assertThat( new FilesystemValueChecker(tsgm, SyscallCache.NO_CACHE, FSVC_THREADS_FOR_TEST) @@ -836,7 +840,8 @@ private void checkActionDirtiedByFile( batchStatter, new ModifiedFileSet.Builder().modify(file.getExecPath()).build(), /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .containsExactly(actionKey); assertThat( new FilesystemValueChecker(tsgm, SyscallCache.NO_CACHE, FSVC_THREADS_FOR_TEST) @@ -847,7 +852,8 @@ private void checkActionDirtiedByFile( .modify(file.getExecPath().getParentDirectory()) .build(), /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .isEmpty(); assertThat( new FilesystemValueChecker(tsgm, SyscallCache.NO_CACHE, FSVC_THREADS_FOR_TEST) @@ -856,7 +862,8 @@ private void checkActionDirtiedByFile( batchStatter, ModifiedFileSet.NOTHING_MODIFIED, /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .isEmpty(); } @@ -904,7 +911,8 @@ public void getDirtyActionValues_touchedTreeDirectory_returnsEmptyDiff( batchStat.getBatchStat(fs), modifiedSet.getModifiedFileSet(tree.getExecPath()), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).isEmpty(); assertThat(modifiedOutputsCaptor.getAllValues()).isEmpty(); @@ -928,7 +936,8 @@ public void getDirtyActionValues_deleteEmptyTreeDirectory_returnsTreeKey( batchStat.getBatchStat(fs), modifiedSet.getModifiedFileSet(tree.getExecPath()), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(tree); @@ -955,7 +964,8 @@ public void getDirtyActionValues_treeDirectoryReplacedWithSymlink_returnsTreeKey batchStat.getBatchStat(fs), ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(tree); @@ -980,7 +990,8 @@ public void getDirtyActionValues_modifiedTreeFile_returnsTreeKey( batchStat.getBatchStat(fs), modifiedSet.getModifiedFileSet(treeFile.getExecPath()), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(treeFile); @@ -1006,7 +1017,8 @@ public void getDirtyActionValues_addedTreeFile_returnsTreeKey( batchStat.getBatchStat(fs), modifiedSet.getModifiedFileSet(newFile.getExecPath()), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionValues).containsExactly(actionKey); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(tree); @@ -1031,7 +1043,8 @@ public void getDirtyActionValues_addedTreeFileToEmptyTree_returnsTreeKey( batchStat.getBatchStat(fs), modifiedSet.getModifiedFileSet(newFile.getExecPath()), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(tree); @@ -1056,7 +1069,8 @@ public void getDirtyActionValues_deletedTreeFile_returnsTreeKey( batchStat.getBatchStat(fs), modifiedSet.getModifiedFileSet(treeFile.getExecPath()), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(treeFile, tree); @@ -1089,7 +1103,8 @@ public void getDirtyActionValues_everythingModified_returnsAllKeys() throws Exce batchStat.getBatchStat(fs), ModifiedFileSet.EVERYTHING_MODIFIED, /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey1, actionKey2); assertThat(modifiedOutputsCaptor.getAllValues()).containsExactly(tree1File, tree2, tree2File); @@ -1125,7 +1140,8 @@ public void getDirtyActionValues_changedFileNotInModifiedSet_returnsKeysFromSetO .modify((reportFirst ? tree1File : tree2File).getExecPath()) .build(), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(reportFirst ? actionKey1 : actionKey2); assertThat(modifiedOutputsCaptor.getAllValues()) @@ -1171,7 +1187,8 @@ public void getDirtyActionValues_middleFileSkippedInModifiedFileSet_returnsKeysF .modify(treeCFile.getExecPath()) .build(), /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).containsExactly(actionKey1, actionKey3); assertThat(modifiedOutputsCaptor.getAllValues()) @@ -1196,7 +1213,8 @@ public void getDirtyActionValues_nothingModified_returnsEmptyDiff() throws Excep batchStat.getBatchStat(fs), ModifiedFileSet.NOTHING_MODIFIED, /*trustRemoteArtifacts=*/ false, - mockModifiedOutputsReceiver); + mockModifiedOutputsReceiver, + /*leaseService=*/ null); assertThat(dirtyActionKeys).isEmpty(); assertThat(modifiedOutputsCaptor.getAllValues()).isEmpty(); @@ -1367,7 +1385,8 @@ public void testRemoteAndLocalArtifacts() throws Exception { /* batchStatter= */ null, ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ true, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .isEmpty(); // Create the "out1" artifact on the filesystem and test that it invalidates the generating @@ -1380,7 +1399,8 @@ public void testRemoteAndLocalArtifacts() throws Exception { /* batchStatter= */ null, ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ true, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .containsExactly(actionKey1); } @@ -1419,7 +1439,8 @@ public void testRemoteAndLocalTreeArtifacts() throws Exception { /* batchStatter= */ null, ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .isEmpty(); // Create dir/foo on the local disk and test that it invalidates the associated sky key. @@ -1432,7 +1453,8 @@ public void testRemoteAndLocalTreeArtifacts() throws Exception { /* batchStatter= */ null, ModifiedFileSet.EVERYTHING_MODIFIED, /* trustRemoteArtifacts= */ false, - (ignored, ignored2) -> {})) + (ignored, ignored2) -> {}, + /*leaseService=*/ null)) .containsExactly(actionKey); }