Take shared repo-gc lock wherever needed

6 files changed, 83 insertions, 8 deletions

diff --git a/src/buildtool/serve_api/serve_service/TARGETS b/src/buildtool/serve_api/serve_service/TARGETS
index 936ccc43..92886167 100644
--- a/src/buildtool/serve_api/serve_service/TARGETS
+++ b/src/buildtool/serve_api/serve_service/TARGETS
@@ -26,6 +26,7 @@
     , ["src/utils/cpp", "expected"]
     , ["src/buildtool/storage", "config"]
     , ["src/buildtool/storage", "storage"]
+    , ["src/buildtool/storage", "repository_garbage_collector"]
     ]
   , "stage": ["src", "buildtool", "serve_api", "serve_service"]
   , "private-deps":
@@ -122,6 +123,7 @@
     , ["src/buildtool/common", "config"]
     , ["src/buildtool/serve_api/remote", "serve_api"]
     , ["src/buildtool/storage", "backend_description"]
+    , ["src/buildtool/storage", "repository_garbage_collector"]
     ]
   }
 , "configuration_service":
diff --git a/src/buildtool/serve_api/serve_service/source_tree.cpp b/src/buildtool/serve_api/serve_service/source_tree.cpp
index 480dfd34..29e9ade2 100644
--- a/src/buildtool/serve_api/serve_service/source_tree.cpp
+++ b/src/buildtool/serve_api/serve_service/source_tree.cpp
@@ -32,6 +32,7 @@
 #include "src/buildtool/multithreading/async_map_utils.hpp"
 #include "src/buildtool/storage/fs_utils.hpp"
 #include "src/buildtool/storage/garbage_collector.hpp"
+#include "src/buildtool/storage/repository_garbage_collector.hpp"
 #include "src/utils/archive/archive_ops.hpp"
 
 namespace {
@@ -197,6 +198,13 @@ auto SourceTreeService::ServeCommitTree(
     ::grpc::ServerContext* /* context */,
     const ::justbuild::just_serve::ServeCommitTreeRequest* request,
     ServeCommitTreeResponse* response) -> ::grpc::Status {
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        logger_->Emit(LogLevel::Error, "Could not acquire repo gc SharedLock");
+        response->set_status(ServeCommitTreeResponse::INTERNAL_ERROR);
+        return ::grpc::Status::OK;
+    }
+
     auto const& commit{request->commit()};
     auto const& subdir{request->subdir()};
     // try in local build root Git cache
@@ -704,6 +712,13 @@ auto SourceTreeService::ServeArchiveTree(
     ::grpc::ServerContext* /* context */,
     const ::justbuild::just_serve::ServeArchiveTreeRequest* request,
     ServeArchiveTreeResponse* response) -> ::grpc::Status {
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        logger_->Emit(LogLevel::Error, "Could not acquire repo gc SharedLock");
+        response->set_status(ServeArchiveTreeResponse::INTERNAL_ERROR);
+        return ::grpc::Status::OK;
+    }
+
     auto const& content{request->content()};
     auto archive_type = ArchiveTypeToString(request->archive_type());
     auto const& subdir{request->subdir()};
@@ -883,6 +898,13 @@ auto SourceTreeService::DistdirImportToGit(
         content_list,
     bool sync_tree,
     ServeDistdirTreeResponse* response) -> ::grpc::Status {
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        logger_->Emit(LogLevel::Error, "Could not acquire repo gc SharedLock");
+        response->set_status(ServeDistdirTreeResponse::INTERNAL_ERROR);
+        return ::grpc::Status::OK;
+    }
+
     // create tmp directory for the distdir
     auto distdir_tmp_dir = storage_config_.CreateTypedTmpDir("distdir");
     if (not distdir_tmp_dir) {
@@ -1271,7 +1293,14 @@ auto SourceTreeService::ServeContent(
     const ::justbuild::just_serve::ServeContentRequest* request,
     ServeContentResponse* response) -> ::grpc::Status {
     auto const& content{request->content()};
-    // acquire lock for CAS
+    // acquire locks
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        logger_->Emit(LogLevel::Error, "Could not acquire repo gc SharedLock");
+        response->set_status(ServeContentResponse::INTERNAL_ERROR);
+        return ::grpc::Status::OK;
+    }
+
     auto lock = GarbageCollector::SharedLock(storage_config_);
     if (!lock) {
         logger_->Emit(LogLevel::Error, "Could not acquire gc SharedLock");
@@ -1380,7 +1409,14 @@ auto SourceTreeService::ServeTree(
     const ::justbuild::just_serve::ServeTreeRequest* request,
     ServeTreeResponse* response) -> ::grpc::Status {
     auto const& tree_id{request->tree()};
-    // acquire lock for CAS
+    // acquire locks
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        logger_->Emit(LogLevel::Error, "Could not acquire repo gc SharedLock");
+        response->set_status(ServeTreeResponse::INTERNAL_ERROR);
+        return ::grpc::Status::OK;
+    }
+
     auto lock = GarbageCollector::SharedLock(storage_config_);
     if (!lock) {
         logger_->Emit(LogLevel::Error, "Could not acquire gc SharedLock");
@@ -1513,7 +1549,14 @@ auto SourceTreeService::CheckRootTree(
     const ::justbuild::just_serve::CheckRootTreeRequest* request,
     CheckRootTreeResponse* response) -> ::grpc::Status {
     auto const& tree_id{request->tree()};
-    // acquire lock for CAS
+    // acquire locks
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        logger_->Emit(LogLevel::Error, "Could not acquire repo gc SharedLock");
+        response->set_status(CheckRootTreeResponse::INTERNAL_ERROR);
+        return ::grpc::Status::OK;
+    }
+
     auto lock = GarbageCollector::SharedLock(storage_config_);
     if (!lock) {
         logger_->Emit(LogLevel::Error, "Could not acquire gc SharedLock");
@@ -1614,13 +1657,14 @@ auto SourceTreeService::GetRemoteTree(
     const ::justbuild::just_serve::GetRemoteTreeRequest* request,
     GetRemoteTreeResponse* response) -> ::grpc::Status {
     auto const& tree_id{request->tree()};
-    // acquire lock for CAS
+    // acquire locks
     auto lock = GarbageCollector::SharedLock(storage_config_);
     if (!lock) {
         logger_->Emit(LogLevel::Error, "Could not acquire gc SharedLock");
         response->set_status(GetRemoteTreeResponse::INTERNAL_ERROR);
         return ::grpc::Status::OK;
     }
+
     // get tree from remote CAS into tmp dir
     auto digest = ArtifactDigest{tree_id, 0, /*is_tree=*/true};
     if (not apis_.remote->IsAvailable(digest)) {
diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp
index 45c78946..c2dd3344 100644
--- a/src/buildtool/serve_api/serve_service/target.cpp
+++ b/src/buildtool/serve_api/serve_service/target.cpp
@@ -43,6 +43,7 @@
 #include "src/buildtool/serve_api/serve_service/target_utils.hpp"
 #include "src/buildtool/storage/backend_description.hpp"
 #include "src/buildtool/storage/garbage_collector.hpp"
+#include "src/buildtool/storage/repository_garbage_collector.hpp"
 #include "src/buildtool/storage/target_cache_key.hpp"
 #include "src/utils/cpp/verify_hash.hpp"
 
@@ -130,10 +131,16 @@ auto TargetService::ServeTarget(
     auto const& target_cache_key_digest =
         ArtifactDigest{request->target_cache_key_id()};
 
-    // acquire lock for CAS
+    // acquire locks
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        auto msg = std::string("Could not acquire repo gc SharedLock");
+        logger_->Emit(LogLevel::Error, msg);
+        return ::grpc::Status{::grpc::StatusCode::INTERNAL, msg};
+    }
     auto lock = GarbageCollector::SharedLock(storage_config_);
     if (!lock) {
-        auto msg = std::string("Could not acquire gc SharedLock");
+        auto msg = std::string("Could not acquire CAS gc SharedLock");
         logger_->Emit(LogLevel::Error, msg);
         return ::grpc::Status{::grpc::StatusCode::INTERNAL, msg};
     }
@@ -728,6 +735,14 @@ auto TargetService::ServeTargetDescription(
     // retrieve content of target file
     std::optional<std::string> target_file_content{std::nullopt};
     bool tree_found{false};
+    // Get repository lock before inspecting the root git cache
+    auto repo_lock = RepositoryGarbageCollector::SharedLock(storage_config_);
+    if (!repo_lock) {
+        auto msg = std::string("Could not acquire repo gc SharedLock");
+        logger_->Emit(LogLevel::Error, msg);
+        return ::grpc::Status{::grpc::StatusCode::INTERNAL, msg};
+    }
+
     // try in local build root Git cache
     if (auto res = GetBlobContent(
             storage_config_.GitRoot(), root_tree, target_file, logger_)) {
diff --git a/src/other_tools/just_mr/TARGETS b/src/other_tools/just_mr/TARGETS
index b3d03483..17e9b248 100644
--- a/src/other_tools/just_mr/TARGETS
+++ b/src/other_tools/just_mr/TARGETS
@@ -14,8 +14,9 @@
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/logging", "log_level"]
     , ["src/buildtool/main", "version"]
-    , ["src/buildtool/storage", "storage"]
     , ["src/buildtool/storage", "config"]
+    , ["src/buildtool/storage", "repository_garbage_collector"]
+    , ["src/buildtool/storage", "storage"]
     , "cli"
     , "exit_codes"
     , "fetch"
@@ -225,10 +226,11 @@
     , ["src/buildtool/build_engine/expression", "expression"]
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/multithreading", "task_system"]
+    , ["src/buildtool/storage", "repository_garbage_collector"]
+    , ["src/utils/cpp", "file_locking"]
     , "exit_codes"
     , "setup"
     , "utils"
-    , ["src/utils/cpp", "file_locking"]
     , "setup_utils"
     ]
   }
diff --git a/src/other_tools/just_mr/launch.cpp b/src/other_tools/just_mr/launch.cpp
index 1d211cd9..642f5f73 100644
--- a/src/other_tools/just_mr/launch.cpp
+++ b/src/other_tools/just_mr/launch.cpp
@@ -25,6 +25,7 @@
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/buildtool/storage/config.hpp"
 #include "src/buildtool/storage/garbage_collector.hpp"
+#include "src/buildtool/storage/repository_garbage_collector.hpp"
 #include "src/other_tools/just_mr/exit_codes.hpp"
 #include "src/other_tools/just_mr/setup.hpp"
 #include "src/other_tools/just_mr/setup_utils.hpp"
@@ -64,6 +65,11 @@ auto CallJust(std::optional<std::filesystem::path> const& config_file,
     if (subcommand and kKnownJustSubcommands.contains(*subcommand)) {
         // Read the config file if needed
         if (kKnownJustSubcommands.at(*subcommand).config) {
+            auto repo_lock =
+                RepositoryGarbageCollector::SharedLock(storage_config);
+            if (not repo_lock) {
+                return kExitGenericFailure;
+            }
             lock = GarbageCollector::SharedLock(storage_config);
             if (not lock) {
                 return kExitGenericFailure;
diff --git a/src/other_tools/just_mr/main.cpp b/src/other_tools/just_mr/main.cpp
index b332da16..55447e54 100644
--- a/src/other_tools/just_mr/main.cpp
+++ b/src/other_tools/just_mr/main.cpp
@@ -37,6 +37,7 @@
 #include "src/buildtool/main/version.hpp"
 #include "src/buildtool/storage/config.hpp"
 #include "src/buildtool/storage/garbage_collector.hpp"
+#include "src/buildtool/storage/repository_garbage_collector.hpp"
 #include "src/other_tools/just_mr/cli.hpp"
 #include "src/other_tools/just_mr/exit_codes.hpp"
 #include "src/other_tools/just_mr/fetch.hpp"
@@ -367,6 +368,11 @@ auto main(int argc, char* argv[]) -> int {
                             forward_build_root,
                             my_name);
         }
+        auto repo_lock =
+            RepositoryGarbageCollector::SharedLock(*storage_config);
+        if (not repo_lock) {
+            return kExitGenericFailure;
+        }
         auto lock = GarbageCollector::SharedLock(*storage_config);
         if (not lock) {
             return kExitGenericFailure;