LargeBlobs: Skip splicing of dependent objects during uplinking of AC, TC and trees.

4 files changed, 73 insertions, 51 deletions

diff --git a/src/buildtool/storage/garbage_collector.cpp b/src/buildtool/storage/garbage_collector.cpp
index 232c8ab8..7c5f9db2 100644
--- a/src/buildtool/storage/garbage_collector.cpp
+++ b/src/buildtool/storage/garbage_collector.cpp
@@ -65,7 +65,11 @@ auto GarbageCollector::GlobalUplinkBlob(bazel_re::Digest const& digest,
         // Note that we uplink with _skip_sync_ as we want to prefer hard links
         // from older generations over copies from the companion file/exec CAS.
         if (Storage::Generation(i).CAS().LocalUplinkBlob(
-                latest_cas, digest, is_executable, /*skip_sync=*/true)) {
+                latest_cas,
+                digest,
+                is_executable,
+                /*skip_sync=*/true,
+                /*splice_result=*/true)) {
             return true;
         }
     }
@@ -91,7 +95,8 @@ auto GarbageCollector::GlobalUplinkTree(bazel_re::Digest const& digest) noexcept
     // Try to find tree in all generations.
     auto const& latest_cas = Storage::Generation(0).CAS();
     for (std::size_t i = 0; i < StorageConfig::NumGenerations(); ++i) {
-        if (Storage::Generation(i).CAS().LocalUplinkTree(latest_cas, digest)) {
+        if (Storage::Generation(i).CAS().LocalUplinkTree(
+                latest_cas, digest, /*splice_result=*/true)) {
             return true;
         }
     }
diff --git a/src/buildtool/storage/local_cas.hpp b/src/buildtool/storage/local_cas.hpp
index 892946de..449878a2 100644
--- a/src/buildtool/storage/local_cas.hpp
+++ b/src/buildtool/storage/local_cas.hpp
@@ -226,13 +226,16 @@ class LocalCAS {
     /// \param digest           The digest of the blob to uplink.
     /// \param is_executable    Uplink blob with executable permissions.
     /// \param skip_sync        Do not sync between file/executable CAS.
+    /// \param splice_result    Create the result of splicing in the latest
+    /// generation.
     /// \returns True if blob was successfully uplinked.
     template <bool kIsLocalGeneration = not kDoGlobalUplink>
     requires(kIsLocalGeneration) [[nodiscard]] auto LocalUplinkBlob(
         LocalGenerationCAS const& latest,
         bazel_re::Digest const& digest,
         bool is_executable,
-        bool skip_sync = false) const noexcept -> bool;
+        bool skip_sync = false,
+        bool splice_result = false) const noexcept -> bool;
 
     /// \brief Uplink tree from this generation to latest LocalCAS generation.
     /// This function is only available for instances that are used as local GC
@@ -245,11 +248,14 @@ class LocalCAS {
     /// \tparam kIsLocalGeneration  True if this instance is a local generation.
     /// \param latest   The latest LocalCAS generation.
     /// \param digest   The digest of the tree to uplink.
+    /// \param splice_result    Create the result of splicing in the latest
+    /// generation.
     /// \returns True if tree was successfully uplinked.
     template <bool kIsLocalGeneration = not kDoGlobalUplink>
     requires(kIsLocalGeneration) [[nodiscard]] auto LocalUplinkTree(
         LocalGenerationCAS const& latest,
-        bazel_re::Digest const& digest) const noexcept -> bool;
+        bazel_re::Digest const& digest,
+        bool splice_result = false) const noexcept -> bool;
 
     /// \brief Uplink large entry from this generation to latest LocalCAS
     /// generation. This function is only available for instances that are used
@@ -309,14 +315,15 @@ class LocalCAS {
     template <bool kIsLocalGeneration = not kDoGlobalUplink>
     requires(kIsLocalGeneration) [[nodiscard]] auto LocalUplinkGitTree(
         LocalGenerationCAS const& latest,
-        bazel_re::Digest const& digest) const noexcept -> bool;
+        bazel_re::Digest const& digest,
+        bool splice_result = false) const noexcept -> bool;
 
     template <bool kIsLocalGeneration = not kDoGlobalUplink>
     requires(kIsLocalGeneration) [[nodiscard]] auto LocalUplinkBazelDirectory(
         LocalGenerationCAS const& latest,
         bazel_re::Digest const& digest,
-        gsl::not_null<std::unordered_set<bazel_re::Digest>*> const& seen)
-        const noexcept -> bool;
+        gsl::not_null<std::unordered_set<bazel_re::Digest>*> const& seen,
+        bool splice_result = false) const noexcept -> bool;
 
     template <ObjectType kType, bool kIsLocalGeneration = not kDoGlobalUplink>
     requires(kIsLocalGeneration) [[nodiscard]] auto TrySplice(
diff --git a/src/buildtool/storage/local_cas.tpp b/src/buildtool/storage/local_cas.tpp
index b1c25504..02c007d5 100644
--- a/src/buildtool/storage/local_cas.tpp
+++ b/src/buildtool/storage/local_cas.tpp
@@ -288,7 +288,8 @@ requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::LocalUplinkBlob(
     LocalGenerationCAS const& latest,
     bazel_re::Digest const& digest,
     bool is_executable,
-    bool skip_sync) const noexcept -> bool {
+    bool skip_sync,
+    bool splice_result) const noexcept -> bool {
     // Determine blob path in latest generation.
     auto blob_path_latest = latest.BlobPathNoSync(digest, is_executable);
     if (blob_path_latest) {
@@ -307,39 +308,42 @@ requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::LocalUplinkBlob(
         return false;
     }
 
-    // Uplink blob from older generation to the latest generation.
-    bool uplinked =
-        blob_path_latest.has_value() or
-        latest.StoreBlob</*kOwner=*/true>(*blob_path, is_executable);
-
-    if (uplinked) {
+    if (spliced) {
         // The result of uplinking of a large object must not affect the
         // result of uplinking in general. In other case, two sequential calls
         // to BlobPath might return different results: The first call splices
         // and uplinks the object, but fails at large entry uplinking. The
         // second call finds the tree in the youngest generation and returns.
         std::ignore = LocalUplinkLargeObject<ObjectType::File>(latest, digest);
+        if (not splice_result) {
+            return true;
+        }
     }
-    return uplinked;
+
+    // Uplink blob from older generation to the latest generation.
+    return blob_path_latest.has_value() or
+           latest.StoreBlob</*kOwner=*/true>(*blob_path, is_executable);
 }
 
 template <bool kDoGlobalUplink>
 template <bool kIsLocalGeneration>
 requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::LocalUplinkTree(
     LocalGenerationCAS const& latest,
-    bazel_re::Digest const& digest) const noexcept -> bool {
+    bazel_re::Digest const& digest,
+    bool splice_result) const noexcept -> bool {
     if (Compatibility::IsCompatible()) {
         std::unordered_set<bazel_re::Digest> seen{};
-        return LocalUplinkBazelDirectory(latest, digest, &seen);
+        return LocalUplinkBazelDirectory(latest, digest, &seen, splice_result);
     }
-    return LocalUplinkGitTree(latest, digest);
+    return LocalUplinkGitTree(latest, digest, splice_result);
 }
 
 template <bool kDoGlobalUplink>
 template <bool kIsLocalGeneration>
 requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::LocalUplinkGitTree(
     LocalGenerationCAS const& latest,
-    bazel_re::Digest const& digest) const noexcept -> bool {
+    bazel_re::Digest const& digest,
+    bool splice_result) const noexcept -> bool {
     // Determine tree path in latest generation.
     auto tree_path_latest = latest.cas_tree_.BlobPath(digest);
     if (tree_path_latest) {
@@ -410,8 +414,7 @@ requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::LocalUplinkGitTree(
         }
     }
 
-    // Uplink tree from older generation to the latest generation.
-    if (latest.cas_tree_.StoreBlobFromFile(*tree_path, /*is owner=*/true)) {
+    if (spliced) {
         // Uplink the large entry afterwards:
         // The result of uplinking of a large object must not affect the
         // result of uplinking in general. In other case, two sequential calls
@@ -419,9 +422,14 @@ requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::LocalUplinkGitTree(
         // and uplinks the object, but fails at large entry uplinking. The
         // second call finds the tree in the youngest generation and returns.
         std::ignore = LocalUplinkLargeObject<ObjectType::Tree>(latest, digest);
-        return true;
+        if (not splice_result) {
+            return true;
+        }
     }
-    return false;
+
+    // Uplink tree from older generation to the latest generation.
+    return latest.cas_tree_.StoreBlobFromFile(*tree_path, /*is owner=*/true)
+        .has_value();
 }
 
 template <bool kDoGlobalUplink>
@@ -430,8 +438,8 @@ requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::
     LocalUplinkBazelDirectory(
         LocalGenerationCAS const& latest,
         bazel_re::Digest const& digest,
-        gsl::not_null<std::unordered_set<bazel_re::Digest>*> const& seen)
-        const noexcept -> bool {
+        gsl::not_null<std::unordered_set<bazel_re::Digest>*> const& seen,
+        bool splice_result) const noexcept -> bool {
     // Skip already uplinked directories
     if (seen->contains(digest)) {
         return true;
@@ -470,23 +478,25 @@ requires(kIsLocalGeneration) auto LocalCAS<kDoGlobalUplink>::
     // Determine bazel directory path in latest generation.
     auto dir_path_latest = latest.cas_tree_.BlobPath(digest);
 
+    if (spliced) {
+        // Uplink the large entry afterwards:
+        // The result of uplinking of a large object must not affect the
+        // result of uplinking in general. In other case, two sequential
+        // calls to TreePath might return different results: The first call
+        // splices and uplinks the object, but fails at large entry
+        // uplinking. The second call finds the tree in the youngest
+        // generation and returns.
+        std::ignore = LocalUplinkLargeObject<ObjectType::Tree>(latest, digest);
+    }
+
+    bool const skip_store = spliced and not splice_result;
     // Uplink bazel directory from older generation to the latest
     // generation.
-    if (dir_path_latest.has_value() or
+    if (skip_store or dir_path_latest.has_value() or
         latest.cas_tree_.StoreBlobFromFile(*dir_path,
                                            /*is_owner=*/true)) {
         try {
             seen->emplace(digest);
-
-            // Uplink the large entry afterwards:
-            // The result of uplinking of a large object must not affect the
-            // result of uplinking in general. In other case, two sequential
-            // calls to TreePath might return different results: The first call
-            // splices and uplinks the object, but fails at large entry
-            // uplinking. The second call finds the tree in the youngest
-            // generation and returns.
-            std::ignore =
-                LocalUplinkLargeObject<ObjectType::Tree>(latest, digest);
             return true;
         } catch (...) {
         }
diff --git a/test/buildtool/storage/large_object_cas.test.cpp b/test/buildtool/storage/large_object_cas.test.cpp
index 7f45c7b8..da348727 100644
--- a/test/buildtool/storage/large_object_cas.test.cpp
+++ b/test/buildtool/storage/large_object_cas.test.cpp
@@ -516,31 +516,31 @@ TEST_CASE_METHOD(HermeticLocalTestFixture,
     auto result_path = cas.TreePath(*large_tree_digest);
     REQUIRE(result_path);
 
-    // The nested tree and all it's large parts must be spliced to the same
-    // locations:
-    CHECK(FileSystemManager::IsFile(*nested_tree_path));
-    CHECK(FileSystemManager::IsFile(*nested_blob_path));
+    // Only the main object must be reconstructed:
     CHECK(FileSystemManager::IsFile(*large_tree_path));
 
-    // Check there are no spliced results in old generations:
-    for (std::size_t i = 1; i < StorageConfig::NumGenerations(); ++i) {
-        auto const& generation_cas = Storage::Generation(i).CAS();
-        REQUIRE_FALSE(generation_cas.TreePath(*nested_tree_digest));
-        REQUIRE_FALSE(generation_cas.TreePath(*large_tree_digest));
-        REQUIRE_FALSE(generation_cas.BlobPath(*nested_blob_digest,
-                                              /*is_executable=*/false));
-    }
+    // It's parts must not be reconstructed by default:
+    CHECK_FALSE(FileSystemManager::IsFile(*nested_tree_path));
+    CHECK_FALSE(FileSystemManager::IsFile(*nested_blob_path));
 
-    // Check large entries are in the latest generation:
     auto const& latest_cas = Storage::Generation(0).CAS();
+
+    // However, they might be reconstructed on request because there entries are
+    // in the latest generation:
     auto split_nested_tree_2 = latest_cas.SplitTree(*nested_tree_digest);
     REQUIRE_FALSE(std::get_if<LargeObjectError>(&split_nested_tree_2));
 
     auto split_nested_blob_2 = latest_cas.SplitBlob(*nested_blob_digest);
     REQUIRE_FALSE(std::get_if<LargeObjectError>(&split_nested_blob_2));
 
-    auto split_large_tree_2 = latest_cas.SplitTree(*large_tree_digest);
-    REQUIRE_FALSE(std::get_if<LargeObjectError>(&split_large_tree_2));
+    // Check there are no spliced results in old generations:
+    for (std::size_t i = 1; i < StorageConfig::NumGenerations(); ++i) {
+        auto const& generation_cas = Storage::Generation(i).CAS();
+        REQUIRE_FALSE(generation_cas.TreePath(*nested_tree_digest));
+        REQUIRE_FALSE(generation_cas.TreePath(*large_tree_digest));
+        REQUIRE_FALSE(generation_cas.BlobPath(*nested_blob_digest,
+                                              /*is_executable=*/false));
+    }
 }
 namespace {