diff --git a/src/com/facebook/buck/artifact_cache/AbstractAsynchronousCache.java b/src/com/facebook/buck/artifact_cache/AbstractAsynchronousCache.java index 545b46ce1c4..00c244058e5 100644 --- a/src/com/facebook/buck/artifact_cache/AbstractAsynchronousCache.java +++ b/src/com/facebook/buck/artifact_cache/AbstractAsynchronousCache.java @@ -26,6 +26,7 @@ import com.facebook.buck.io.filesystem.ProjectFilesystem; import com.facebook.buck.log.Logger; import com.facebook.buck.util.Scope; +import com.facebook.buck.util.types.Pair; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; @@ -402,6 +403,84 @@ public final ListenableFuture store(ArtifactInfo info, BorrowablePath outp }); } + @Override + public final ListenableFuture store( + ImmutableList> artifacts) { + if (!getCacheReadMode().isWritable()) { + return Futures.immediateFuture(null); + } + + ImmutableList.Builder> matchedArtifactsBuilder = + ImmutableList.builderWithExpectedSize(artifacts.size()); + ImmutableList.Builder artifactSizesInBytesBuilder = + ImmutableList.builderWithExpectedSize(artifacts.size()); + + for (int i = 0; i < artifacts.size(); i++) { + BorrowablePath output = artifacts.get(i).getSecond(); + ArtifactInfo info = artifacts.get(i).getFirst(); + long artifactSizeBytes = getFileSize(output.getPath()); + if (artifactExceedsMaximumSize(artifactSizeBytes)) { + LOG.info( + "Artifact too big so not storing it in the %s cache. file=[%s] buildTarget=[%s]", + name, output.getPath(), info.getBuildTarget()); + continue; + } + + Path tmp; + try { + tmp = getPathForArtifact(output); + } catch (IOException e) { + LOG.error(e, "Failed to store artifact in temp file: " + output.getPath()); + continue; + } + + matchedArtifactsBuilder.add(new Pair<>(info, tmp)); + artifactSizesInBytesBuilder.add(artifactSizeBytes); + } + + ImmutableList> matchedArtifacts = matchedArtifactsBuilder.build(); + + if (matchedArtifacts.isEmpty()) { + return Futures.immediateFuture(null); + } + + ImmutableList artifactSizesInBytes = artifactSizesInBytesBuilder.build(); + + ImmutableList.Builder eventsBuilder = + ImmutableList.builderWithExpectedSize(artifactSizesInBytes.size()); + for (int i = 0; i < artifactSizesInBytes.size(); i++) { + eventsBuilder.add( + eventListener.storeScheduled( + matchedArtifacts.get(i).getFirst(), artifactSizesInBytes.get(i))); + } + + ImmutableList events = eventsBuilder.build(); + + return storeExecutorService.submit( + () -> { + for (int i = 0; i < matchedArtifacts.size(); i++) { + StoreEvents.StoreRequestEvents requestEvents = events.get(i).started(); + try { + StoreResult result = + storeImpl( + matchedArtifacts.get(i).getFirst(), matchedArtifacts.get(i).getSecond()); + requestEvents.finished(result); + } catch (IOException e) { + String msg = + String.format( + "store(%s): %s: %s", + matchedArtifacts.get(i).getFirst().getRuleKeys(), + e.getClass().getName(), + e.getMessage()); + requestEvents.failed(e, msg); + throw new RuntimeException(e); + } + } + + return null; + }); + } + @Override public final ListenableFuture deleteAsync(List ruleKeys) { if (!getCacheReadMode().isWritable()) { diff --git a/src/com/facebook/buck/artifact_cache/ArtifactCache.java b/src/com/facebook/buck/artifact_cache/ArtifactCache.java index b0a27d378bd..4f46ea3b719 100644 --- a/src/com/facebook/buck/artifact_cache/ArtifactCache.java +++ b/src/com/facebook/buck/artifact_cache/ArtifactCache.java @@ -21,9 +21,13 @@ import com.facebook.buck.core.rulekey.RuleKey; import com.facebook.buck.io.file.BorrowablePath; import com.facebook.buck.io.file.LazyPath; +import com.facebook.buck.util.types.Pair; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; import java.util.List; import javax.annotation.Nullable; @@ -59,6 +63,31 @@ ListenableFuture fetchAsync( */ ListenableFuture store(ArtifactInfo info, BorrowablePath output); + /** + * Store the list of artifacts at path specified by output to cache in passed order, such that it + * can later be fetched using ruleKey as the lookup key. If any internal errors occur, fail + * silently and continue execution. Store may be performed synchronously or asynchronously. + * + *

This is a noop if {@link #getCacheReadMode()}} returns {@code READONLY}. + * + * @param artifacts list of artifact info and path to be uploaded to the cache in given order. + * @return {@link ListenableFuture} that completes once the store has finished. + */ + default ListenableFuture store( + ImmutableList> artifacts) { + if (artifacts.isEmpty()) { + return Futures.immediateFuture(null); + } + + Pair first = artifacts.get(0); + ImmutableList> rest = artifacts.subList(1, artifacts.size()); + + return Futures.transformAsync( + this.store(first.getFirst(), first.getSecond()), + input -> this.store(rest), + MoreExecutors.directExecutor()); + } + /** * Check if the cache contains the given artifacts, keyed by ruleKeys, without fetching them, and * return a map of results wrapped in a {@link ListenableFuture}. This is supposed to be fast, but diff --git a/src/com/facebook/buck/artifact_cache/MultiArtifactCache.java b/src/com/facebook/buck/artifact_cache/MultiArtifactCache.java index 6ef6153cb15..4f4e523a44f 100644 --- a/src/com/facebook/buck/artifact_cache/MultiArtifactCache.java +++ b/src/com/facebook/buck/artifact_cache/MultiArtifactCache.java @@ -21,6 +21,7 @@ import com.facebook.buck.core.rulekey.RuleKey; import com.facebook.buck.io.file.BorrowablePath; import com.facebook.buck.io.file.LazyPath; +import com.facebook.buck.util.types.Pair; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; @@ -130,6 +131,30 @@ public ListenableFuture store(ArtifactInfo info, BorrowablePath output) { return storeToCaches(writableArtifactCaches, info, output); } + @Override + public ListenableFuture store(ImmutableList> artifacts) { + if (writableArtifactCaches.size() != 1) { + ImmutableList.Builder> artifactTemporaryPaths = + ImmutableList.builderWithExpectedSize(artifacts.size()); + for (int i = 0; i < artifacts.size(); i++) { + artifactTemporaryPaths.add( + new Pair<>( + artifacts.get(i).getFirst(), + BorrowablePath.notBorrowablePath(artifacts.get(i).getSecond().getPath()))); + } + artifacts = artifactTemporaryPaths.build(); + } + + List> storeFutures = + Lists.newArrayListWithExpectedSize(writableArtifactCaches.size()); + for (ArtifactCache artifactCache : writableArtifactCaches) { + storeFutures.add(artifactCache.store(artifacts)); + } + + // Aggregate future to ensure all store operations have completed. + return Futures.transform(Futures.allAsList(storeFutures), Functions.constant(null)); + } + @Override public ListenableFuture> multiContainsAsync( ImmutableSet ruleKeys) { diff --git a/src/com/facebook/buck/artifact_cache/NoopArtifactCache.java b/src/com/facebook/buck/artifact_cache/NoopArtifactCache.java index ec9353421d7..594fc5f908f 100644 --- a/src/com/facebook/buck/artifact_cache/NoopArtifactCache.java +++ b/src/com/facebook/buck/artifact_cache/NoopArtifactCache.java @@ -22,6 +22,7 @@ import com.facebook.buck.core.rulekey.RuleKey; import com.facebook.buck.io.file.BorrowablePath; import com.facebook.buck.io.file.LazyPath; +import com.facebook.buck.util.types.Pair; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -49,6 +50,11 @@ public ListenableFuture store(ArtifactInfo info, BorrowablePath output) { return Futures.immediateFuture(null); } + @Override + public ListenableFuture store(ImmutableList> artifacts) { + return Futures.immediateFuture(null); + } + @Override public ListenableFuture> multiContainsAsync( ImmutableSet ruleKeys) { diff --git a/src/com/facebook/buck/artifact_cache/RetryingCacheDecorator.java b/src/com/facebook/buck/artifact_cache/RetryingCacheDecorator.java index d083adc2ac5..d42b860db2f 100644 --- a/src/com/facebook/buck/artifact_cache/RetryingCacheDecorator.java +++ b/src/com/facebook/buck/artifact_cache/RetryingCacheDecorator.java @@ -26,7 +26,9 @@ import com.facebook.buck.io.file.LazyPath; import com.facebook.buck.log.Logger; import com.facebook.buck.slb.NoHealthyServersException; +import com.facebook.buck.util.types.Pair; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.Futures; @@ -110,6 +112,11 @@ public ListenableFuture store(ArtifactInfo info, BorrowablePath output) { return delegate.store(info, output); } + @Override + public ListenableFuture store(ImmutableList> artifacts) { + return delegate.store(artifacts); + } + @Override public ListenableFuture> multiContainsAsync( ImmutableSet ruleKeys) { diff --git a/src/com/facebook/buck/artifact_cache/TwoLevelArtifactCacheDecorator.java b/src/com/facebook/buck/artifact_cache/TwoLevelArtifactCacheDecorator.java index 40ed405888e..e6df94c4a36 100644 --- a/src/com/facebook/buck/artifact_cache/TwoLevelArtifactCacheDecorator.java +++ b/src/com/facebook/buck/artifact_cache/TwoLevelArtifactCacheDecorator.java @@ -30,6 +30,7 @@ import com.facebook.buck.io.filesystem.ProjectFilesystem; import com.facebook.buck.log.Logger; import com.facebook.buck.util.RichStream; +import com.facebook.buck.util.types.Pair; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; @@ -249,17 +250,27 @@ private ListenableFuture attemptTwoLevelStore(ArtifactInfo info, Borrow .putAll(info.getMetadata()) .put(METADATA_KEY, hashCode) .build(); + // We need to upload artifacts in this order to prevent race condition. If we would do + // it concurrently it is possible that we upload metadata before the file. Then other + // builder read metadata, but cannot find a file (which is still being uploaded), and + // decide that we have to re-upload it. With enough machines building the same target + // we end up with constant re-uploading and rebuilding flow. The following issue is + // only in case when output hash changes between builds. + Pair artifact = + new Pair<>(ArtifactInfo.builder().addRuleKeys(new RuleKey(hashCode)).build(), output); + Pair metadata = + new Pair<>( + ArtifactInfo.builder() + .setRuleKeys(info.getRuleKeys()) + .setMetadata(metadataWithCacheKey) + .build(), + BorrowablePath.notBorrowablePath(emptyFilePath)); return Futures.transform( - Futures.allAsList( - delegate.store( - ArtifactInfo.builder() - .setRuleKeys(info.getRuleKeys()) - .setMetadata(metadataWithCacheKey) - .build(), - BorrowablePath.notBorrowablePath(emptyFilePath)), - delegate.store( - ArtifactInfo.builder().addRuleKeys(new RuleKey(hashCode)).build(), output)), + // This relies on the fact that delegate stores artifacts in sequential way in the order + // they are being passed. If we store them internally in consecutive way, there is a + // possibility of race condition. + delegate.store(ImmutableList.of(artifact, metadata)), Functions.constant(true), MoreExecutors.directExecutor()); } diff --git a/test/com/facebook/buck/artifact_cache/DirArtifactCacheTest.java b/test/com/facebook/buck/artifact_cache/DirArtifactCacheTest.java index 08334e00099..2e19feda9b7 100644 --- a/test/com/facebook/buck/artifact_cache/DirArtifactCacheTest.java +++ b/test/com/facebook/buck/artifact_cache/DirArtifactCacheTest.java @@ -47,6 +47,7 @@ import com.facebook.buck.testutil.TemporaryPaths; import com.facebook.buck.util.DirectoryCleaner; import com.facebook.buck.util.cache.FileHashCache; +import com.facebook.buck.util.types.Pair; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -386,6 +387,119 @@ public void testCacheStoresAndFetchHits() throws IOException { } } + @Test + public void testCacheMultiStoresAndContainsHits() throws IOException { + Path cacheDir = tmpDir.newFolder(); + Path fileX = tmpDir.newFile("x"); + Path fileY = tmpDir.newFile("y"); + Path fileZ = tmpDir.newFile("z"); + + fileHashCache = + new FakeFileHashCache( + ImmutableMap.of( + fileX, HashCode.fromInt(0), + fileY, HashCode.fromInt(1), + fileZ, HashCode.fromInt(2))); + + dirArtifactCache = + new DirArtifactCache( + "dir", + TestProjectFilesystems.createProjectFilesystem(cacheDir), + Paths.get("."), + CacheReadMode.READWRITE, + /* maxCacheSizeBytes */ Optional.empty()); + + Files.write(fileX, "x".getBytes(UTF_8)); + Files.write(fileY, "y".getBytes(UTF_8)); + Files.write(fileZ, "x".getBytes(UTF_8)); + + BuildRule inputRuleX = new BuildRuleForTest(fileX); + BuildRule inputRuleY = new BuildRuleForTest(fileY); + BuildRule inputRuleZ = new BuildRuleForTest(fileZ); + assertFalse(inputRuleX.equals(inputRuleY)); + assertFalse(inputRuleX.equals(inputRuleZ)); + assertFalse(inputRuleY.equals(inputRuleZ)); + ActionGraphBuilder graphBuilder = new TestActionGraphBuilder(); + graphBuilder.addToIndex(inputRuleX); + graphBuilder.addToIndex(inputRuleY); + graphBuilder.addToIndex(inputRuleZ); + SourcePathRuleFinder ruleFinder = new SourcePathRuleFinder(graphBuilder); + SourcePathResolver resolver = DefaultSourcePathResolver.from(ruleFinder); + + DefaultRuleKeyFactory fakeRuleKeyFactory = + new TestDefaultRuleKeyFactory(fileHashCache, resolver, ruleFinder); + + RuleKey ruleKeyX = fakeRuleKeyFactory.build(inputRuleX); + RuleKey ruleKeyY = fakeRuleKeyFactory.build(inputRuleY); + RuleKey ruleKeyZ = fakeRuleKeyFactory.build(inputRuleZ); + + assertEquals( + CacheResultType.MISS, + Futures.getUnchecked( + dirArtifactCache.fetchAsync(null, ruleKeyX, LazyPath.ofInstance(fileX))) + .getType()); + assertEquals( + CacheResultType.MISS, + Futures.getUnchecked( + dirArtifactCache.fetchAsync(null, ruleKeyY, LazyPath.ofInstance(fileY))) + .getType()); + assertEquals( + CacheResultType.MISS, + Futures.getUnchecked( + dirArtifactCache.fetchAsync(null, ruleKeyZ, LazyPath.ofInstance(fileZ))) + .getType()); + + dirArtifactCache.store( + ImmutableList.of( + new Pair<>( + ArtifactInfo.builder().addRuleKeys(ruleKeyX).build(), + BorrowablePath.notBorrowablePath(fileX)), + new Pair<>( + ArtifactInfo.builder().addRuleKeys(ruleKeyY).build(), + BorrowablePath.notBorrowablePath(fileY)), + new Pair<>( + ArtifactInfo.builder().addRuleKeys(ruleKeyZ).build(), + BorrowablePath.notBorrowablePath(fileZ)))); + + Files.delete(fileX); + Files.delete(fileY); + Files.delete(fileZ); + + assertEquals( + CacheResultType.HIT, + Futures.getUnchecked( + dirArtifactCache.fetchAsync(null, ruleKeyX, LazyPath.ofInstance(fileX))) + .getType()); + assertEquals( + CacheResultType.HIT, + Futures.getUnchecked( + dirArtifactCache.fetchAsync(null, ruleKeyY, LazyPath.ofInstance(fileY))) + .getType()); + assertEquals( + CacheResultType.HIT, + Futures.getUnchecked( + dirArtifactCache.fetchAsync(null, ruleKeyZ, LazyPath.ofInstance(fileZ))) + .getType()); + + assertEquals(inputRuleX, new BuildRuleForTest(fileX)); + assertEquals(inputRuleY, new BuildRuleForTest(fileY)); + assertEquals(inputRuleZ, new BuildRuleForTest(fileZ)); + + ImmutableList cachedFiles = ImmutableList.copyOf(dirArtifactCache.getAllFilesInCache()); + assertEquals(6, cachedFiles.size()); + + ImmutableSet filenames = + cachedFiles + .stream() + .map(input -> input.getFileName().toString()) + .collect(ImmutableSet.toImmutableSet()); + + for (RuleKey ruleKey : ImmutableSet.of(ruleKeyX, ruleKeyY, ruleKeyZ)) { + assertThat(filenames, Matchers.hasItem(ruleKey.toString())); + assertThat(filenames, Matchers.hasItem(ruleKey + ".metadata")); + } + } + @Test public void testCacheStoresAndMultiContainsHits() throws IOException { Path cacheDir = tmpDir.newFolder(); diff --git a/test/com/facebook/buck/artifact_cache/HttpArtifactCacheTest.java b/test/com/facebook/buck/artifact_cache/HttpArtifactCacheTest.java index 1c21b26c596..74879f5a813 100644 --- a/test/com/facebook/buck/artifact_cache/HttpArtifactCacheTest.java +++ b/test/com/facebook/buck/artifact_cache/HttpArtifactCacheTest.java @@ -29,13 +29,16 @@ import com.facebook.buck.event.BuckEventBus; import com.facebook.buck.event.ConsoleEvent; import com.facebook.buck.event.DefaultBuckEventBus; +import com.facebook.buck.io.file.BorrowablePath; import com.facebook.buck.io.file.LazyPath; import com.facebook.buck.slb.HttpResponse; import com.facebook.buck.slb.HttpService; import com.facebook.buck.slb.OkHttpResponseWrapper; import com.facebook.buck.testutil.FakeProjectFilesystem; import com.facebook.buck.util.timing.IncrementingFakeClock; +import com.facebook.buck.util.types.Pair; import com.google.common.base.Charsets; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.io.ByteSource; @@ -441,6 +444,54 @@ public void testStoreMultipleKeys() throws Exception { cache.close(); } + @Test + public void testMulitStore() throws Exception { + RuleKey ruleKey1 = new RuleKey("00000000000000000000000000000000"); + RuleKey ruleKey2 = new RuleKey("11111111111111111111111111111111"); + String data = "data"; + FakeProjectFilesystem filesystem = new FakeProjectFilesystem(); + Path output = Paths.get("output/file"); + filesystem.writeContentsToPath(data, output); + Set stored = new HashSet<>(); + argsBuilder.setProjectFilesystem(filesystem); + argsBuilder.setStoreClient( + withMakeRequest( + ((path, requestBuilder) -> { + Request request = requestBuilder.url(SERVER).build(); + Buffer buf = new Buffer(); + request.body().writeTo(buf); + try (DataInputStream in = + new DataInputStream(new ByteArrayInputStream(buf.readByteArray()))) { + int keys = in.readInt(); + for (int i = 0; i < keys; i++) { + stored.add(new RuleKey(in.readUTF())); + } + } + Response response = + new Response.Builder() + .body(createDummyBody()) + .code(HttpURLConnection.HTTP_ACCEPTED) + .protocol(Protocol.HTTP_1_1) + .request(request) + .message("") + .build(); + return new OkHttpResponseWrapper(response); + }))); + HttpArtifactCache cache = new HttpArtifactCache(argsBuilder.build()); + cache + .store( + ImmutableList.of( + new Pair<>( + ArtifactInfo.builder().addRuleKeys(ruleKey1).build(), + BorrowablePath.notBorrowablePath(output)), + new Pair<>( + ArtifactInfo.builder().addRuleKeys(ruleKey2).build(), + BorrowablePath.notBorrowablePath(output)))) + .get(); + assertThat(stored, Matchers.containsInAnyOrder(ruleKey1, ruleKey2)); + cache.close(); + } + @Test public void testFetchWrongKey() { FakeProjectFilesystem filesystem = new FakeProjectFilesystem(); diff --git a/test/com/facebook/buck/core/build/engine/impl/CachingBuildEngineTest.java b/test/com/facebook/buck/core/build/engine/impl/CachingBuildEngineTest.java index 294ada38a31..d63efb58be3 100644 --- a/test/com/facebook/buck/core/build/engine/impl/CachingBuildEngineTest.java +++ b/test/com/facebook/buck/core/build/engine/impl/CachingBuildEngineTest.java @@ -160,6 +160,7 @@ import com.facebook.buck.util.json.ObjectMappers; import com.facebook.buck.util.timing.DefaultClock; import com.facebook.buck.util.timing.IncrementingFakeClock; +import com.facebook.buck.util.types.Pair; import com.facebook.buck.util.zip.CustomZipEntry; import com.facebook.buck.util.zip.CustomZipOutputStream; import com.facebook.buck.util.zip.ZipConstants; @@ -4528,6 +4529,12 @@ public ListenableFuture store(ArtifactInfo info, BorrowablePath output) { throw new UnsupportedOperationException(); } + @Override + public ListenableFuture store( + ImmutableList> artifacts) { + throw new UnsupportedOperationException(); + } + @Override public ListenableFuture> multiContainsAsync( ImmutableSet ruleKeys) {