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 2c5dd6891dac77..4590850868df42 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 @@ -382,11 +382,13 @@ private void saveFile(Digest digest, Store store, InputStream in) throws IOExcep Path temp = getTempPath(); try { - try (FileOutputStream out = new FileOutputStream(temp.getPathFile())) { + try (OutputStream out = temp.getOutputStream()) { ByteStreams.copy(in, out); // Fsync temp before we rename it to avoid data loss in the case of machine // crashes (the OS may reorder the writes and the rename). - out.getFD().sync(); + if (out instanceof FileOutputStream fos) { + fos.getFD().sync(); + } } path.getParentDirectory().createDirectoryAndParents(); temp.renameTo(path); diff --git a/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD b/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD index 15f574ed9281a6..e088c67d070fbc 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD @@ -20,15 +20,21 @@ java_test( deps = [ "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", + "//src/main/java/com/google/devtools/build/lib/remote:store", "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", "//src/main/java/com/google/devtools/build/lib/remote/disk", "//src/main/java/com/google/devtools/build/lib/remote/util", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs/bazel", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/test/java/com/google/devtools/build/lib:test_runner", + "//third_party:error_prone_annotations", "//third_party:guava", "//third_party:junit4", "//third_party:mockito", + "//third_party:truth", + "//third_party/protobuf:protobuf_java", "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", ], ) diff --git a/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java index 9e7532c015b63b..057d879dc1e6b5 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java @@ -13,22 +13,45 @@ // limitations under the License. package com.google.devtools.build.lib.remote.disk; +import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertThrows; +import static org.junit.Assume.assumeNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import build.bazel.remote.execution.v2.ActionResult; +import build.bazel.remote.execution.v2.Digest; +import build.bazel.remote.execution.v2.Directory; +import build.bazel.remote.execution.v2.FileNode; +import build.bazel.remote.execution.v2.OutputDirectory; +import build.bazel.remote.execution.v2.OutputFile; import build.bazel.remote.execution.v2.RequestMetadata; +import build.bazel.remote.execution.v2.Tree; +import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; +import com.google.devtools.build.lib.remote.Store; +import com.google.devtools.build.lib.remote.common.CacheNotFoundException; +import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; +import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.SyscallCache; +import com.google.devtools.build.lib.vfs.bazel.BazelHashFunctions; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.protobuf.ByteString; +import com.google.protobuf.Message; +import java.io.IOException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import org.junit.Before; @@ -45,7 +68,7 @@ public class DiskCacheClientTest { MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(1)); private final FileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256); - private final Path root = fs.getPath("/"); + private final Path root = fs.getPath("/disk_cache"); private DiskCacheClient client; private RemoteActionExecutionContext context; @@ -60,7 +83,7 @@ public void setUp() throws Exception { } @Test - public void testSpawnCheckingCacheEvent() throws Exception { + public void downloadActionResult_reportsSpawnCheckingCacheEvent() throws Exception { var unused = getFromFuture( client.downloadActionResult( @@ -70,4 +93,343 @@ public void testSpawnCheckingCacheEvent() throws Exception { verify(context.getSpawnExecutionContext()).report(SpawnCheckingCacheEvent.create("disk-cache")); } + + @Test + public void findMissingDigests_returnsAllDigests() throws Exception { + ImmutableList digests = + ImmutableList.of(getDigest("foo"), getDigest("bar"), getDigest("baz")); + assertThat(getFromFuture(client.findMissingDigests(context, digests))) + .containsExactlyElementsIn(digests); + } + + @Test + public void toPath_forCas_forOldStyleHashFunction() throws Exception { + Digest digest = Digest.newBuilder().setHash("0123456789abcdef").setSizeBytes(42).build(); + + Path path = client.toPath(digest, Store.CAS); + + assertThat(path).isEqualTo(root.getRelative("cas/01/0123456789abcdef")); + } + + @Test + public void toPath_forAc_forOldStyleHashFunction() throws Exception { + Digest digest = Digest.newBuilder().setHash("0123456789abcdef").setSizeBytes(42).build(); + + Path path = client.toPath(digest, Store.AC); + + assertThat(path).isEqualTo(root.getRelative("ac/01/0123456789abcdef")); + } + + @Test + public void toPath_forCas_forNewStyleHashFunction() throws Exception { + assumeNotNull(BazelHashFunctions.BLAKE3); // BLAKE3 not available in Blaze. + + DiskCacheClient client = + new DiskCacheClient( + root, + new DigestUtil(SyscallCache.NO_CACHE, BazelHashFunctions.BLAKE3), + executorService, + /* verifyDownloads= */ true); + Digest digest = Digest.newBuilder().setHash("0123456789abcdef").setSizeBytes(42).build(); + Path path = client.toPath(digest, Store.CAS); + + assertThat(path).isEqualTo(root.getRelative("blake3/cas/01/0123456789abcdef")); + } + + @Test + public void toPath_forAc_forNewStyleHashFunction() throws Exception { + assumeNotNull(BazelHashFunctions.BLAKE3); // BLAKE3 not available in Blaze. + + DiskCacheClient client = + new DiskCacheClient( + root, + new DigestUtil(SyscallCache.NO_CACHE, BazelHashFunctions.BLAKE3), + executorService, + /* verifyDownloads= */ true); + Digest digest = Digest.newBuilder().setHash("0123456789abcdef").setSizeBytes(42).build(); + Path path = client.toPath(digest, Store.AC); + + assertThat(path).isEqualTo(root.getRelative("blake3/ac/01/0123456789abcdef")); + } + + @Test + public void uploadFile_whenMissing_populatesCas() throws Exception { + assertThat(root.exists()).isTrue(); + Path file = fs.getPath("/file"); + FileSystemUtils.writeContent(file, UTF_8, "contents"); + Digest digest = getDigest("contents"); + + var unused = getFromFuture(client.uploadFile(context, digest, file)); + + assertThat(FileSystemUtils.readContent(getCasPath(digest), UTF_8)).isEqualTo("contents"); + } + + @Test + public void uploadFile_whenPresent_updatesMtime() throws Exception { + Path file = fs.getPath("/file"); + FileSystemUtils.writeContent(file, UTF_8, "contents"); + Digest digest = getDigest("contents"); + + // The contents would match under normal operation. This serves to check that we don't + // unnecessarily overwrite the file. + Path path = populateCas(digest, "existing contents"); + + var unused = getFromFuture(client.uploadFile(context, digest, file)); + + assertThat(FileSystemUtils.readContent(path, UTF_8)).isEqualTo("existing contents"); + assertThat(path.getLastModifiedTime()).isNotEqualTo(0); + } + + @Test + public void uploadBlob_whenMissing_populatesCas() throws Exception { + ByteString blob = ByteString.copyFromUtf8("contents"); + Digest digest = getDigest("contents"); + + var unused = getFromFuture(client.uploadBlob(context, digest, blob)); + + assertThat(FileSystemUtils.readContent(getCasPath(digest), UTF_8)).isEqualTo("contents"); + } + + @Test + public void uploadBlob_whenPresent_updatesMtime() throws Exception { + ByteString blob = ByteString.copyFromUtf8("contents"); + Digest digest = getDigest("contents"); + + // The contents would match under normal operation. This serves to check that we don't + // unnecessarily overwrite the file. + Path path = populateCas(digest, "existing contents"); + + var unused = getFromFuture(client.uploadBlob(context, digest, blob)); + + assertThat(FileSystemUtils.readContent(path, UTF_8)).isEqualTo("existing contents"); + assertThat(path.getLastModifiedTime()).isNotEqualTo(0); + } + + @Test + public void uploadActionResult_whenMissing_populatesAc() throws Exception { + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = ActionResult.newBuilder().setExitCode(42).build(); + Path path = getAcPath(actionKey); + + var unused = getFromFuture(client.uploadActionResult(context, actionKey, actionResult)); + + assertThat(FileSystemUtils.readContent(path)).isEqualTo(actionResult.toByteArray()); + } + + @Test + public void uploadActionResult_whenPresent_updatesMtime() throws Exception { + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = ActionResult.newBuilder().setExitCode(42).build(); + + Path path = populateAc(actionKey, actionResult); + + // The contents would match under normal operation. This serves to check that we don't + // unnecessarily overwrite the file. + var unused = + getFromFuture( + client.uploadActionResult(context, actionKey, ActionResult.getDefaultInstance())); + + assertThat(FileSystemUtils.readContent(path)).isEqualTo(actionResult.toByteArray()); + assertThat(path.getLastModifiedTime()).isNotEqualTo(0); + } + + @Test + public void downloadBlob_whenPresent_returnsContents() throws Exception { + Digest digest = getDigest("contents"); + populateCas(digest, "contents"); + Path out = fs.getPath("/out"); + + var unused = getFromFuture(client.downloadBlob(context, digest, out.getOutputStream())); + + assertThat(FileSystemUtils.readContent(out, UTF_8)).isEqualTo("contents"); + } + + @Test + public void downloadBlob_whenMissing_throwsCacheNotFoundException() throws Exception { + Path out = fs.getPath("/out"); + + assertThrows( + CacheNotFoundException.class, + () -> + getFromFuture( + client.downloadBlob(context, getDigest("contents"), out.getOutputStream()))); + } + + @Test + public void downloadBlob_whenCorrupted_throwsOutputDigestMismatchException() throws Exception { + Digest digest = getDigest("contents"); + populateCas(digest, "corrupted contents"); + Path out = fs.getPath("/out"); + + assertThrows( + OutputDigestMismatchException.class, + () -> getFromFuture(client.downloadBlob(context, digest, out.getOutputStream()))); + } + + @Test + public void downloadActionResult_whenPresent_returnsCachedActionResult() throws Exception { + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = ActionResult.newBuilder().setExitCode(42).build(); + + Path path = populateAc(actionKey, actionResult); + + CachedActionResult result = + getFromFuture(client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false)); + + assertThat(result).isEqualTo(CachedActionResult.disk(actionResult)); + assertThat(path.getLastModifiedTime()).isNotEqualTo(0); + } + + @Test + public void downloadActionResult_whenMissing_returnsNull() throws Exception { + ActionKey actionKey = new ActionKey(getDigest("key")); + + CachedActionResult result = + getFromFuture(client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false)); + + assertThat(result).isNull(); + } + + @Test + public void downloadActionResult_withReferencedBlobsPresent_updatesMtimeOnBlobs() + throws Exception { + Digest stdoutDigest = getDigest("stdout contents"); + Digest stderrDigest = getDigest("stderr contents"); + Digest fileDigest = getDigest("file contents"); + Digest treeFileDigest = getDigest("tree file contents"); + Tree tree = getTreeWithFile(treeFileDigest); + Digest treeDigest = getDigest(tree); + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = + ActionResult.newBuilder() + .setStdoutDigest(stdoutDigest) + .setStderrDigest(stderrDigest) + .addOutputFiles(OutputFile.newBuilder().setDigest(fileDigest).build()) + .addOutputDirectories(OutputDirectory.newBuilder().setTreeDigest(getDigest(tree))) + .build(); + + Path acPath = populateAc(actionKey, actionResult); + Path stdoutCasPath = populateCas(stdoutDigest, "stdout contents"); + Path stderrCasPath = populateCas(stderrDigest, "stderr contents"); + Path fileCasPath = populateCas(fileDigest, "file contents"); + Path treeCasPath = populateCas(treeDigest, tree); + Path treeFileCasPath = populateCas(treeFileDigest, "tree file contents"); + + CachedActionResult result = + getFromFuture(client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false)); + + assertThat(result).isEqualTo(CachedActionResult.disk(actionResult)); + assertThat(acPath.getLastModifiedTime()).isNotEqualTo(0); + assertThat(stdoutCasPath.getLastModifiedTime()).isNotEqualTo(0); + assertThat(stderrCasPath.getLastModifiedTime()).isNotEqualTo(0); + assertThat(fileCasPath.getLastModifiedTime()).isNotEqualTo(0); + assertThat(treeCasPath.getLastModifiedTime()).isNotEqualTo(0); + assertThat(treeFileCasPath.getLastModifiedTime()).isNotEqualTo(0); + } + + @Test + public void downloadActionResult_withReferencedFileMissing_returnsNull() throws Exception { + Digest fileDigest = getDigest("contents"); + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = + ActionResult.newBuilder() + .addOutputFiles(OutputFile.newBuilder().setDigest(fileDigest).build()) + .build(); + + populateAc(actionKey, actionResult); + + CachedActionResult result = + getFromFuture(client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false)); + + assertThat(result).isNull(); + } + + @Test + public void downloadActionResult_withReferencedTreeMissing_returnsNull() throws Exception { + Digest fileDigest = getDigest("contents"); + Tree tree = getTreeWithFile(fileDigest); + Digest treeDigest = getDigest(tree); + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = + ActionResult.newBuilder() + .addOutputDirectories(OutputDirectory.newBuilder().setTreeDigest(treeDigest)) + .build(); + + populateAc(actionKey, actionResult); + populateCas(fileDigest, "contents"); + + CachedActionResult result = + getFromFuture(client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false)); + + assertThat(result).isNull(); + } + + @Test + public void downloadActionResult_withReferencedTreeFileMissing_returnsNull() throws Exception { + Digest fileDigest = getDigest("contents"); + Tree tree = getTreeWithFile(fileDigest); + Digest treeDigest = getDigest(tree); + ActionKey actionKey = new ActionKey(getDigest("key")); + ActionResult actionResult = + ActionResult.newBuilder() + .addOutputDirectories(OutputDirectory.newBuilder().setTreeDigest(treeDigest)) + .build(); + + populateAc(actionKey, actionResult); + populateCas(treeDigest, tree); + + CachedActionResult result = + getFromFuture(client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false)); + + assertThat(result).isNull(); + } + + private Tree getTreeWithFile(Digest fileDigest) { + return Tree.newBuilder() + .addChildren(Directory.newBuilder().addFiles(FileNode.newBuilder().setDigest(fileDigest))) + .build(); + } + + private Path getCasPath(Digest digest) { + return client.toPath(digest, Store.CAS); + } + + @CanIgnoreReturnValue + private Path populateCas(Digest digest, String contents) throws IOException { + return populateCas(digest, contents.getBytes(UTF_8)); + } + + @CanIgnoreReturnValue + private Path populateCas(Digest digest, Message m) throws IOException { + return populateCas(digest, m.toByteArray()); + } + + private Path populateCas(Digest digest, byte[] contents) throws IOException { + Path path = getCasPath(digest); + path.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContent(path, contents); + path.setLastModifiedTime(0); + return path; + } + + private Path getAcPath(ActionKey actionKey) { + return client.toPath(actionKey.getDigest(), Store.AC); + } + + @CanIgnoreReturnValue + private Path populateAc(ActionKey actionKey, ActionResult actionResult) throws IOException { + Path path = getAcPath(actionKey); + path.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContent(path, actionResult.toByteArray()); + path.setLastModifiedTime(0); + return path; + } + + private Digest getDigest(String contents) { + return DIGEST_UTIL.computeAsUtf8(contents); + } + + private Digest getDigest(Message m) { + return DIGEST_UTIL.compute(m.toByteArray()); + } }