SEARCH-300 (BTS-789) remove database directory for arangosearch (aran…

…godb#18420) * SEARCH-300 remove database directory and a lot of refactoring * Fix * Fix * Fix * Fix review suggestions * Fix review suggestions * Fix review suggestions * Fix review suggestions * Add test * Fixes * Fix test
leonardoadame · Mar 24, 2023 · f9ccaee · f9ccaee
1 parent 7fd66b4
commit f9ccaee
Show file tree

Hide file tree

Showing 21 changed files with 759 additions and 1,004 deletions.
diff --git a/arangod/Cluster/HeartbeatThread.cpp b/arangod/Cluster/HeartbeatThread.cpp
@@ -1352,7 +1352,7 @@ bool HeartbeatThread::handlePlanChangeCoordinator(uint64_t currentPlanVersion) {
           TRI_ASSERT(false);
           dbName = "n/a";
         }
-        Result res = databaseFeature.dropDatabase(id, true);
+        Result res = databaseFeature.dropDatabase(id);
         events::DropDatabase(dbName, res, ExecContext::current());
       }
     }

diff --git a/arangod/IResearch/IResearchDataStore.cpp b/arangod/IResearch/IResearchDataStore.cpp
@@ -57,12 +57,12 @@
 #include <store/store_utils.hpp>
 #include <utils/encryption.hpp>
 #include <utils/singleton.hpp>
-#include <utils/file_utils.hpp>
 #include <chrono>
 #include <string>
 
 #include <absl/cleanup/cleanup.h>
 #include <absl/strings/str_cat.h>
+#include <filesystem>
 
 using namespace std::literals;
 
@@ -1048,13 +1048,14 @@ void IResearchDataStore::shutdownDataStore() noexcept {
 
 Result IResearchDataStore::deleteDataStore() noexcept {
   shutdownDataStore();
-  bool exists;
+  std::error_code error;
+  std::filesystem::remove_all(_dataStore._path, error);
   // remove persisted data store directory if present
-  if (!irs::file_utils::exists_directory(exists, _dataStore._path.c_str()) ||
-      (exists && !irs::file_utils::remove(_dataStore._path.c_str()))) {
-    return {TRI_ERROR_INTERNAL,
-            absl::StrCat("failed to remove ArangoSearch index '",
-                         index().id().id(), "'")};
+  if (error) {
+    return {
+        TRI_ERROR_INTERNAL,
+        absl::StrCat("failed to remove ArangoSearch index '", index().id().id(),
+                     "' with error code '", error.message(), "'")};
   }
   return {};
 }
@@ -1216,21 +1217,31 @@ Result IResearchDataStore::initDataStore(
 
   _dataStore._path = getPersistedPath(dbPathFeature, *this);
 
-  // Must manually ensure that the data store directory exists (since not
-  // using a lockfile)
-  if (!irs::file_utils::exists_directory(pathExists,
-                                         _dataStore._path.c_str()) ||
-      (!pathExists &&
-       !irs::file_utils::mkdir(_dataStore._path.c_str(), true))) {
+  // Must manually ensure that the data store directory exists
+  // (since not using a lockfile)
+  std::error_code error;
+  pathExists = std::filesystem::exists(_dataStore._path, error);
+  auto createResult = [&](std::string_view what) -> Result {
     return {TRI_ERROR_CANNOT_CREATE_DIRECTORY,
-            absl::StrCat("Failed to create data store directory with path '",
+            absl::StrCat(what, " failed for data store directory with path '",
                          _dataStore._path.string(),
                          "' while initializing ArangoSearch index '",
-                         index().id().id(), "'")};
+                         index().id().id(), "' with error '", error.message(),
+                         "'")};
+  };
+  if (error) {
+    pathExists = true;  // we don't want to remove directory in unknown state
+    return createResult("Exists");
+  }
+  if (!pathExists) {
+    std::filesystem::create_directories(_dataStore._path, error);
+    if (error) {
+      return createResult("Create");
+    }
   }
 
   _dataStore._directory = std::make_unique<irs::MMapDirectory>(
-      _dataStore._path.u8string(),
+      _dataStore._path,
       initCallback ? initCallback() : irs::directory_attributes{});
 
   switch (_engine->recoveryState()) {
@@ -1957,17 +1968,13 @@ void IResearchDataStore::finishCreation() {
 ////////////////////////////////////////////////////////////////////////////////
 std::filesystem::path getPersistedPath(DatabasePathFeature const& dbPathFeature,
                                        IResearchDataStore const& dataStore) {
-  std::filesystem::path dataPath{dbPathFeature.directory()};
-
-  dataPath /= "databases";
   auto& i = dataStore.index();
-  dataPath /= absl::StrCat("database-", i.collection().vocbase().id());
-  dataPath /=
+  auto path = getPersistedPath(dbPathFeature, i.collection().vocbase());
+  path /=
       absl::StrCat(StaticStrings::ViewArangoSearchType, "-",
                    // has to be 'id' since this can be a per-shard collection
                    i.collection().id().id(), "_", i.id().id());
-
-  return dataPath;
+  return path;
 }
 
 template Result

diff --git a/arangod/IResearch/IResearchDataStore.h b/arangod/IResearch/IResearchDataStore.h
@@ -517,7 +517,7 @@ class IResearchDataStore {
 };
 
 std::filesystem::path getPersistedPath(DatabasePathFeature const& dbPathFeature,
-                                       IResearchDataStore const& link);
+                                       IResearchDataStore const& dataStore);
 
 }  // namespace iresearch
 }  // namespace arangodb
diff --git a/arangod/IResearch/IResearchFeature.cpp b/arangod/IResearch/IResearchFeature.cpp
@@ -1246,6 +1246,27 @@ void IResearchFeature::unprepare() {
   _running.store(false);
 }
 
+std::filesystem::path getPersistedPath(DatabasePathFeature const& dbPathFeature,
+                                       TRI_vocbase_t& database) {
+  std::filesystem::path path{dbPathFeature.directory()};
+  path /= "databases";
+  path /= absl::StrCat("database-", database.id());
+  return path;
+}
+
+void cleanupDatabase(TRI_vocbase_t& database) {
+  auto const& feature = database.server().getFeature<DatabasePathFeature>();
+  auto path = getPersistedPath(feature, database);
+  std::error_code error;
+  std::filesystem::remove_all(path);
+  if (error) {
+    LOG_TOPIC("bad02", ERR, TOPIC)
+        << "Failed to remove arangosearch path for database (id '"
+        << database.id() << "' name: '" << database.name() << "') with error '"
+        << error.message() << "'";
+  }
+}
+
 void IResearchFeature::reportRecoveryProgress(arangodb::IndexId id,
                                               std::string_view phase,
                                               size_t current, size_t total) {

diff --git a/arangod/IResearch/IResearchFeature.h b/arangod/IResearch/IResearchFeature.h
@@ -37,6 +37,7 @@
 #include <string>
 #include <string_view>
 #include <vector>
+#include <filesystem>
 
 namespace arangodb {
 struct IndexTypeFactory;
@@ -91,6 +92,11 @@ inline bool isOffsetInfo(aql::AstNode const& node) noexcept {
          isOffsetInfo(*static_cast<aql::Function const*>(node.getData()));
 }
 
+std::filesystem::path getPersistedPath(DatabasePathFeature const& dbPathFeature,
+                                       TRI_vocbase_t& database);
+
+void cleanupDatabase(TRI_vocbase_t& database);
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @class IResearchFeature
 ////////////////////////////////////////////////////////////////////////////////
@@ -100,13 +106,13 @@ class IResearchFeature final : public ArangodFeature {
 
   explicit IResearchFeature(Server& server);
 
-  void beginShutdown() override;
-  void collectOptions(std::shared_ptr<options::ProgramOptions>) override;
-  void prepare() override;
-  void start() override;
-  void stop() override;
-  void unprepare() override;
-  void validateOptions(std::shared_ptr<options::ProgramOptions>) override;
+  void beginShutdown() final;
+  void collectOptions(std::shared_ptr<options::ProgramOptions>) final;
+  void prepare() final;
+  void start() final;
+  void stop() final;
+  void unprepare() final;
+  void validateOptions(std::shared_ptr<options::ProgramOptions>) final;
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief report progress during recovery phase