diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java index 8f91a182befc2c..6b020a472d85cc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java @@ -45,6 +45,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Predicate; import javax.annotation.Nullable; /** @@ -76,6 +77,9 @@ public interface ActionCache { */ void remove(String key); + /** Removes entry from cache that matches the predicate. */ + void removeIf(Predicate predicate); + /** * An entry in the ActionCache that contains all action input and output * artifact paths and their metadata plus action key itself. @@ -246,7 +250,8 @@ public RemoteFileArtifactValue getOutputFile(Artifact output) { return outputFileMetadata.get(output.getExecPathString()); } - Map getOutputFiles() { + /** Gets metadata of all output files */ + public Map getOutputFiles() { return outputFileMetadata; } @@ -273,7 +278,8 @@ public SerializableTreeArtifactValue getOutputTree(SpecialArtifact output) { return outputTreeMetadata.get(output.getExecPathString()); } - Map getOutputTrees() { + /** Gets metadata of all output trees */ + public Map getOutputTrees() { return outputTreeMetadata; } 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 aac16acc51751e..d906498770e072 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 @@ -55,6 +55,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Predicate; import javax.annotation.Nullable; /** @@ -342,6 +343,10 @@ public ActionCache.Entry get(String key) { return null; } byte[] data = map.get(index); + return get(data); + } + + private ActionCache.Entry get(byte[] data) { try { return data != null ? decode(indexer, data) : null; } catch (IOException e) { @@ -381,6 +386,11 @@ public void remove(String key) { map.remove(indexer.getIndex(key)); } + @Override + public void removeIf(Predicate predicate) { + map.entrySet().removeIf(entry -> predicate.test(get(entry.getValue()))); + } + @ThreadSafety.ThreadHostile @Override public long save() throws IOException { 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 862216b2a87e4a..dca53cd3745a3b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -232,8 +232,11 @@ java_library( srcs = ["LeaseService.java"], deps = [ "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", + "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", "//src/main/java/com/google/devtools/build/skyframe", "//third_party:jsr305", ], diff --git a/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java b/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java index 8479d787c99dc0..0e1d65d628e645 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/LeaseService.java @@ -13,15 +13,13 @@ // limitations under the License. package com.google.devtools.build.lib.remote; -import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.ActionCacheUtils; import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionLookupData; -import com.google.devtools.build.lib.actions.ActionLookupValue; -import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.ActionCache; +import com.google.devtools.build.lib.skyframe.ActionExecutionValue; +import com.google.devtools.build.lib.skyframe.SkyFunctions; +import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.skyframe.MemoizingEvaluator; -import java.util.HashMap; import java.util.Set; import javax.annotation.Nullable; @@ -41,36 +39,44 @@ public void handleMissingInputs(Set missingActionInputs) { return; } - var actions = new HashMap(); + // If any outputs are evicted, remove all remote metadata from skyframe and local action cache. + // + // With TTL based discarding and lease extension, remote cache eviction error won't happen if + // remote cache can guarantee the TTL. However, if it happens, it usually means the remote cache + // is under high load and it could possibly evict more blobs that Bazel wouldn't aware of. + // Following builds could still fail for the same error (caused by different blobs). - try { - for (ActionInput actionInput : missingActionInputs) { - if (actionInput instanceof Artifact.DerivedArtifact) { - Artifact.DerivedArtifact output = (Artifact.DerivedArtifact) actionInput; - ActionLookupData actionLookupData = output.getGeneratingActionKey(); - var actionLookupValue = - memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey()); - if (actionLookupValue instanceof ActionLookupValue) { - Action action = - ((ActionLookupValue) actionLookupValue) - .getAction(actionLookupData.getActionIndex()); - actions.put(actionLookupData, action); + memoizingEvaluator.delete( + key -> { + if (key.functionName().equals(SkyFunctions.ACTION_EXECUTION)) { + try { + var value = memoizingEvaluator.getExistingValue(key); + return value instanceof ActionExecutionValue + && isRemote((ActionExecutionValue) value); + } catch (InterruptedException ignored) { + return false; + } } - } - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + return false; + }); + + if (actionCache != null) { + actionCache.removeIf( + entry -> !entry.getOutputFiles().isEmpty() || !entry.getOutputTrees().isEmpty()); } + } - if (!actions.isEmpty()) { - var actionKeys = actions.keySet(); - memoizingEvaluator.delete(key -> key instanceof ActionLookupData && actionKeys.contains(key)); + private boolean isRemote(ActionExecutionValue value) { + return value.getAllFileValues().values().stream().anyMatch(FileArtifactValue::isRemote) + || value.getAllTreeArtifactValues().values().stream().anyMatch(this::isRemoteTree); + } - if (actionCache != null) { - for (var action : actions.values()) { - ActionCacheUtils.removeCacheEntry(actionCache, action); - } - } - } + private boolean isRemoteTree(TreeArtifactValue treeArtifactValue) { + return treeArtifactValue.getChildValues().values().stream() + .anyMatch(FileArtifactValue::isRemote) + || treeArtifactValue + .getArchivedRepresentation() + .map(ar -> ar.archivedFileValue().isRemote()) + .orElse(false); } } 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 50f7df78317148..2eea1d7e4d81ad 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 @@ -73,6 +73,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; @@ -1255,6 +1256,11 @@ public void remove(String key) { delegate.remove(key); } + @Override + public void removeIf(Predicate predicate) { + delegate.removeIf(predicate); + } + @Override public long save() throws IOException { return delegate.save(); diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java index 93fd34f15b3960..ae8f8312b2b4f6 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java @@ -17,6 +17,7 @@ import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics; import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics.MissReason; import java.io.PrintStream; +import java.util.function.Predicate; /** * Utilities for tests that use the action cache. @@ -38,6 +39,9 @@ public Entry get(String fingerprint) { @Override public void remove(String key) {} + @Override + public void removeIf(Predicate predicate) {} + @Override public long save() { return -1; diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 8d481e9526e72f..b4d8cd57768353 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java @@ -580,6 +580,11 @@ public synchronized void remove(String key) { actionCache.remove(key); } + @Override + public void removeIf(java.util.function.Predicate predicate) { + actionCache.values().removeIf(predicate); + } + public synchronized void reset() { actionCache.clear(); }