diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index 4b973e371d91b2..0eb013512b2704 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -123,6 +123,14 @@ public boolean isRemote() { return false; } + /** + * Returns {@code true} if the file is remote and is available remotely at time {@code epochMilli} + * which is the milliseconds since epoch. + */ + public boolean isRemotelyAvailable(long epochMilli) { + return false; + } + /** * Provides a best-effort determination whether the file was changed since the digest was * computed. This method performs file system I/O, so may be expensive. It's primarily intended to @@ -551,21 +559,23 @@ public static class RemoteFileArtifactValue extends FileArtifactValue { protected final long size; protected final int locationIndex; protected final String actionId; + protected final long expiredAtEpochMilli; - private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) { + private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId, long expiredAtEpochMilli) { this.digest = Preconditions.checkNotNull(digest, actionId); this.size = size; this.locationIndex = locationIndex; this.actionId = actionId; + this.expiredAtEpochMilli = expiredAtEpochMilli; } public static RemoteFileArtifactValue create( - byte[] digest, long size, int locationIndex, String actionId) { - return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); + byte[] digest, long size, int locationIndex, String actionId, long expiredAtEpochMilli) { + return new RemoteFileArtifactValue(digest, size, locationIndex, actionId, expiredAtEpochMilli); } - public static RemoteFileArtifactValue create(byte[] digest, long size, int locationIndex) { - return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ ""); + public static RemoteFileArtifactValue create(byte[] digest, long size, int locationIndex, long expiredAtEpochMilli) { + return new RemoteFileArtifactValue(digest, size, locationIndex, /* actionId= */ "", expiredAtEpochMilli); } public static RemoteFileArtifactValue create( @@ -573,12 +583,13 @@ public static RemoteFileArtifactValue create( long size, int locationIndex, String actionId, + long expiredAtEpochMilli, @Nullable PathFragment materializationExecPath) { if (materializationExecPath != null) { return new RemoteFileArtifactValueWithMaterializationPath( - digest, size, locationIndex, actionId, materializationExecPath); + digest, size, locationIndex, actionId, expiredAtEpochMilli, materializationExecPath); } - return new RemoteFileArtifactValue(digest, size, locationIndex, actionId); + return new RemoteFileArtifactValue(digest, size, locationIndex, actionId, expiredAtEpochMilli); } @Override @@ -591,12 +602,13 @@ public boolean equals(Object o) { return Arrays.equals(digest, that.digest) && size == that.size && locationIndex == that.locationIndex - && Objects.equals(actionId, that.actionId); + && Objects.equals(actionId, that.actionId) + && expiredAtEpochMilli == that.expiredAtEpochMilli; } @Override public int hashCode() { - return Objects.hash(Arrays.hashCode(digest), size, locationIndex, actionId); + return Objects.hash(Arrays.hashCode(digest), size, locationIndex, actionId, expiredAtEpochMilli); } @Override @@ -624,6 +636,10 @@ public String getActionId() { return actionId; } + public long getExpiredAtEpochMilli() { + return expiredAtEpochMilli; + } + @Override public long getModifiedTime() { throw new UnsupportedOperationException( @@ -645,6 +661,11 @@ public boolean isRemote() { return true; } + @Override + public boolean isRemotelyAvailable(long epochMilli) { + return epochMilli < expiredAtEpochMilli; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -652,6 +673,7 @@ public String toString() { .add("size", size) .add("locationIndex", locationIndex) .add("actionId", actionId) + .add("expiredAtEpochMilli", expiredAtEpochMilli) .toString(); } } @@ -671,8 +693,9 @@ private RemoteFileArtifactValueWithMaterializationPath( long size, int locationIndex, String actionId, + long expiredAtEpochMilli, PathFragment materializationExecPath) { - super(digest, size, locationIndex, actionId); + super(digest, size, locationIndex, actionId, expiredAtEpochMilli); this.materializationExecPath = Preconditions.checkNotNull(materializationExecPath); } 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 b2d44b8065e970..dc547aafa90209 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 @@ -504,7 +504,7 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( } return RemoteFileArtifactValue.create( - digest, size, locationIndex, actionId, materializationExecPath); + digest, size, locationIndex, actionId, 0, materializationExecPath); } /** @return action data encoded as a byte[] array. */ 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 b7f31149f9f46f..7f958ba9db78a7 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 @@ -113,12 +113,12 @@ public void updateContext(MetadataInjector metadataInjector) { this.metadataInjector = metadataInjector; } - void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId) + void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId, long expireAtEpochMilli) throws IOException { if (!isOutput(path)) { return; } - remoteOutputTree.injectRemoteFile(path, digest, size, actionId); + remoteOutputTree.injectRemoteFile(path, digest, size, actionId, expireAtEpochMilli); } void flush() throws IOException { @@ -242,6 +242,7 @@ private boolean maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact ou metadata.getSize(), metadata.getLocationIndex(), metadata.getActionId(), + metadata.getExpiredAtEpochMilli(), // Avoid a double indirection when the target is already materialized as a symlink. metadata.getMaterializationExecPath().orElse(targetPath.relativeTo(execRoot))); @@ -256,7 +257,8 @@ private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) remoteFile.getFastDigest(), remoteFile.getSize(), /* locationIndex= */ 1, - remoteFile.getActionId()); + remoteFile.getActionId(), + remoteFile.getExpireAtEpochMilli()); } @Override @@ -713,7 +715,7 @@ protected FileInfo newFile(Clock clock, PathFragment path) { return new RemoteFileInfo(clock); } - void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId) + void injectRemoteFile(PathFragment path, byte[] digest, long size, String actionId, long expireAtEpochMilli) throws IOException { createDirectoryAndParents(path.getParentDirectory()); InMemoryContentInfo node = getOrCreateWritableInode(path); @@ -724,7 +726,7 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size, String action } RemoteFileInfo remoteFileInfo = (RemoteFileInfo) node; - remoteFileInfo.set(digest, size, actionId); + remoteFileInfo.set(digest, size, actionId, expireAtEpochMilli); } @Nullable @@ -742,15 +744,17 @@ static class RemoteFileInfo extends FileInfo { private byte[] digest; private long size; private String actionId; + private long expireAtEpochMilli; RemoteFileInfo(Clock clock) { super(clock); } - private void set(byte[] digest, long size, String actionId) { + private void set(byte[] digest, long size, String actionId, long expireAtEpochMilli) { this.digest = digest; this.size = size; this.actionId = actionId; + this.expireAtEpochMilli = expireAtEpochMilli; } @Override @@ -781,5 +785,9 @@ public long getSize() { public String getActionId() { return actionId; } + + public long getExpireAtEpochMilli() { + return expireAtEpochMilli; + } } } 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 ffa3871a369970..41b3a0469d502d 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 @@ -759,7 +759,7 @@ private void createSymlinks(Iterable symlinks) throws IOExcepti } } - private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata metadata) + private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata metadata, long expireAtEpochMilli) throws IOException { FileSystem actionFileSystem = action.getSpawnExecutionContext().getActionFileSystem(); checkState(actionFileSystem instanceof RemoteActionFileSystem); @@ -780,7 +780,8 @@ private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata met file.path().asFragment(), DigestUtil.toBinaryDigest(file.digest()), file.digest().getSizeBytes(), - context.getRequestMetadata().getActionId()); + context.getRequestMetadata().getActionId(), + expireAtEpochMilli); } } @@ -789,7 +790,8 @@ private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata met file.path().asFragment(), DigestUtil.toBinaryDigest(file.digest()), file.digest().getSizeBytes(), - context.getRequestMetadata().getActionId()); + context.getRequestMetadata().getActionId(), + expireAtEpochMilli); } } @@ -1118,7 +1120,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re } } - injectRemoteArtifacts(action, metadata); + injectRemoteArtifacts(action, metadata, 0); try (SilentCloseable c = Profiler.instance().profile("Remote.downloadInMemoryOutput")) { if (inMemoryOutput != null) { 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..f108acbfb6cad8 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 @@ -58,6 +58,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; @@ -228,6 +229,7 @@ public NavigableSet get() { } }); + long epochMilli = Instant.now().toEpochMilli(); boolean interrupted; try (SilentCloseable c = Profiler.instance().profile("getDirtyActionValues.stat_files")) { for (List> shard : outputShards) { @@ -239,7 +241,8 @@ public NavigableSet get() { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver) + modifiedOutputsReceiver, + epochMilli) : batchStatJob( dirtyKeys, shard, @@ -247,7 +250,8 @@ public NavigableSet get() { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver); + modifiedOutputsReceiver, + epochMilli); executor.execute(job); } @@ -273,7 +277,8 @@ private Runnable batchStatJob( ImmutableSet knownModifiedOutputFiles, Supplier> sortedKnownModifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + long epochMilli) { return () -> { Map> fileToKeyAndValue = new HashMap<>(); Map> treeArtifactsToKeyAndValue = @@ -331,7 +336,8 @@ && shouldCheckFile(knownModifiedOutputFiles, artifact)) { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver) + modifiedOutputsReceiver, + epochMilli) .run(); return; } catch (InterruptedException e) { @@ -393,7 +399,8 @@ private Runnable outputStatJob( ImmutableSet knownModifiedOutputFiles, Supplier> sortedKnownModifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + long epochMilli) { return new Runnable() { @Override public void run() { @@ -405,7 +412,8 @@ public void run() { knownModifiedOutputFiles, sortedKnownModifiedOutputFiles, trustRemoteArtifacts, - modifiedOutputsReceiver)) { + modifiedOutputsReceiver, + epochMilli)) { dirtyKeys.add(keyAndValue.getFirst()); } } @@ -441,7 +449,8 @@ private boolean artifactIsDirtyWithDirectSystemCalls( ImmutableSet knownModifiedOutputFiles, boolean trustRemoteArtifacts, Map.Entry entry, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + long epochMilli) { Artifact file = entry.getKey(); FileArtifactValue lastKnownData = entry.getValue(); if (file.isMiddlemanArtifact() || !shouldCheckFile(knownModifiedOutputFiles, file)) { @@ -453,7 +462,8 @@ private boolean artifactIsDirtyWithDirectSystemCalls( boolean trustRemoteValue = fileMetadata.getType() == FileStateType.NONEXISTENT && lastKnownData.isRemote() - && trustRemoteArtifacts; + && trustRemoteArtifacts + && lastKnownData.isRemotelyAvailable(epochMilli); if (!trustRemoteValue && fileMetadata.couldBeModifiedSince(lastKnownData)) { modifiedOutputsReceiver.reportModifiedOutputFile( fileMetadata.getType() != FileStateType.NONEXISTENT @@ -475,11 +485,12 @@ private boolean actionValueIsDirtyWithDirectSystemCalls( ImmutableSet knownModifiedOutputFiles, Supplier> sortedKnownModifiedOutputFiles, boolean trustRemoteArtifacts, - ModifiedOutputsReceiver modifiedOutputsReceiver) { + ModifiedOutputsReceiver modifiedOutputsReceiver, + long epochMilli) { boolean isDirty = false; for (Map.Entry entry : actionValue.getAllFileValues().entrySet()) { if (artifactIsDirtyWithDirectSystemCalls( - knownModifiedOutputFiles, trustRemoteArtifacts, entry, modifiedOutputsReceiver)) { + knownModifiedOutputFiles, trustRemoteArtifacts, entry, modifiedOutputsReceiver, epochMilli)) { isDirty = true; } } @@ -495,7 +506,8 @@ private boolean actionValueIsDirtyWithDirectSystemCalls( knownModifiedOutputFiles, trustRemoteArtifacts, childEntry, - modifiedOutputsReceiver)) { + modifiedOutputsReceiver, + epochMilli)) { isDirty = true; } } @@ -510,7 +522,8 @@ private boolean actionValueIsDirtyWithDirectSystemCalls( Maps.immutableEntry( archivedRepresentation.archivedTreeFileArtifact(), archivedRepresentation.archivedFileValue()), - modifiedOutputsReceiver)) + modifiedOutputsReceiver, + epochMilli)) .orElse(false); }