From 62386d901ab481c3403b13ebaf9752642e25625c Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 19 Sep 2023 08:10:13 -0700 Subject: [PATCH] Use a self-describing enum instead of a boolean to distinguish the AC and CAS in disk cache operations. PiperOrigin-RevId: 566636073 Change-Id: Iee06b3e84b35fdfd49c2e1d98a03e2531ad9cff8 --- .../remote/disk/DiskAndRemoteCacheClient.java | 3 +- .../lib/remote/disk/DiskCacheClient.java | 56 ++++++++++++------- 2 files changed, 37 insertions(+), 22 deletions(-) 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 d759e907db2630..00c506b660c1d9 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 @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.remote.common.LazyFileOutputStream; 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.disk.DiskCacheClient.Store; import com.google.devtools.build.lib.vfs.Path; import com.google.protobuf.ByteString; import java.io.IOException; @@ -172,7 +173,7 @@ public ListenableFuture downloadBlob( (unused) -> { try { tempOut.close(); - diskCache.captureFile(tempPath, digest, /* isActionCache= */ false); + diskCache.captureFile(tempPath, digest, Store.CAS); } catch (IOException e) { return immediateFailedFuture(e); } 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 65364186d14176..e24cbc7ceeb2aa 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 @@ -48,8 +48,24 @@ /** A on-disk store for the remote action cache. */ public class DiskCacheClient implements RemoteCacheClient { - private static final String AC_DIRECTORY = "ac"; - private static final String CAS_DIRECTORY = "cas"; + /** Identifies one of the two stores comprised by the disk cache. */ + public enum Store { + /** Action cache. */ + AC("ac"), + + /** Content-addressed storage. */ + CAS("cas"); + + private final String name; + + private Store(String name) { + this.name = name; + } + + private String getDirectoryName() { + return name; + } + } private final Path root; private final boolean verifyDownloads; @@ -77,22 +93,22 @@ public DiskCacheClient(Path root, boolean verifyDownloads, DigestUtil digestUtil /** Returns {@code true} if the provided {@code key} is stored in the CAS. */ public boolean contains(Digest digest) { - return toPath(digest.getHash(), /* actionResult= */ false).exists(); + return toPath(digest.getHash(), Store.CAS).exists(); } /** Returns {@link Path} into the CAS for the given {@link Digest}. */ public Path getPath(Digest digest) { - return toPath(digest.getHash(), /* actionResult= */ false); + return toPath(digest.getHash(), Store.CAS); } - public void captureFile(Path src, Digest digest, boolean isActionCache) throws IOException { - Path target = toPath(digest.getHash(), isActionCache); + public void captureFile(Path src, Digest digest, Store store) throws IOException { + Path target = toPath(digest.getHash(), store); target.getParentDirectory().createDirectoryAndParents(); src.renameTo(target); } - private ListenableFuture download(Digest digest, OutputStream out, boolean isActionCache) { - Path p = toPath(digest.getHash(), isActionCache); + private ListenableFuture download(Digest digest, OutputStream out, Store store) { + Path p = toPath(digest.getHash(), store); if (!p.exists()) { return Futures.immediateFailedFuture(new CacheNotFoundException(digest)); } else { @@ -111,7 +127,7 @@ public ListenableFuture downloadBlob( @Nullable DigestOutputStream digestOut = verifyDownloads ? digestUtil.newDigestOutputStream(out) : null; return Futures.transformAsync( - download(digest, digestOut != null ? digestOut : out, /* isActionCache= */ false), + download(digest, digestOut != null ? digestOut : out, Store.CAS), (v) -> { try { if (digestOut != null) { @@ -131,7 +147,7 @@ private void checkDigestExists(Digest digest) throws CacheNotFoundException { return; } - if (!toPath(digest.getHash(), /* actionResult= */ false).exists()) { + if (!toPath(digest.getHash(), Store.CAS).exists()) { throw new CacheNotFoundException(digest); } } @@ -151,7 +167,7 @@ private void checkActionResult(ActionResult actionResult) throws IOException { var treeDigest = outputDirectory.getTreeDigest(); checkDigestExists(treeDigest); - var treePath = toPath(treeDigest.getHash(), /* actionResult= */ false); + var treePath = toPath(treeDigest.getHash(), Store.CAS); var tree = Tree.parseFrom(treePath.getInputStream(), ExtensionRegistryLite.getEmptyRegistry()); checkOutputDirectory(tree.getRoot()); @@ -179,8 +195,7 @@ public ListenableFuture getAuthority() { public ListenableFuture downloadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { return Futures.transformAsync( - Utils.downloadAsActionResult( - actionKey, (digest, out) -> download(digest, out, /* isActionCache= */ true)), + Utils.downloadAsActionResult(actionKey, (digest, out) -> download(digest, out, Store.AC)), actionResult -> { if (actionResult == null) { return immediateFuture(null); @@ -201,7 +216,7 @@ public ListenableFuture downloadActionResult( public ListenableFuture uploadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, ActionResult actionResult) { try (InputStream data = actionResult.toByteString().newInput()) { - saveFile(actionKey.getDigest().getHash(), data, /* actionResult= */ true); + saveFile(actionKey.getDigest().getHash(), data, Store.AC); return immediateFuture(null); } catch (IOException e) { return Futures.immediateFailedFuture(e); @@ -215,7 +230,7 @@ public void close() {} public ListenableFuture uploadFile( RemoteActionExecutionContext context, Digest digest, Path file) { try (InputStream in = file.getInputStream()) { - saveFile(digest.getHash(), in, /* actionResult= */ false); + saveFile(digest.getHash(), in, Store.CAS); } catch (IOException e) { return Futures.immediateFailedFuture(e); } @@ -226,7 +241,7 @@ public ListenableFuture uploadFile( public ListenableFuture uploadBlob( RemoteActionExecutionContext context, Digest digest, ByteString data) { try (InputStream in = data.newInput()) { - saveFile(digest.getHash(), in, /* actionResult= */ false); + saveFile(digest.getHash(), in, Store.CAS); } catch (IOException e) { return Futures.immediateFailedFuture(e); } @@ -245,14 +260,13 @@ protected Path toPathNoSplit(String key) { return root.getChild(key); } - protected Path toPath(String key, boolean actionResult) { - String cacheFolder = actionResult ? AC_DIRECTORY : CAS_DIRECTORY; + protected Path toPath(String key, Store store) { // Create the file in a subfolder to bypass possible folder file count limits - return root.getChild(cacheFolder).getChild(key.substring(0, 2)).getChild(key); + return root.getChild(store.getDirectoryName()).getChild(key.substring(0, 2)).getChild(key); } - private void saveFile(String key, InputStream in, boolean actionResult) throws IOException { - Path target = toPath(key, actionResult); + private void saveFile(String key, InputStream in, Store store) throws IOException { + Path target = toPath(key, store); if (target.exists()) { return; }