GetInfo simplification (#1621)

* Removed force dir in the GetInfo functions
This commit is contained in:
andrejtonev 2024-02-26 15:55:45 +01:00 committed by GitHub
parent 6a4ef55e90
commit 82c47ee80d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 71 additions and 60 deletions

View File

@ -110,9 +110,9 @@ class Database {
* @param force_directory Use the configured directory, do not try to decipher the multi-db version
* @return DatabaseInfo
*/
DatabaseInfo GetInfo(bool force_directory, replication_coordination_glue::ReplicationRole replication_role) const {
DatabaseInfo GetInfo(replication_coordination_glue::ReplicationRole replication_role) const {
DatabaseInfo info;
info.storage_info = storage_->GetInfo(force_directory, replication_role);
info.storage_info = storage_->GetInfo(replication_role);
info.triggers = trigger_store_.GetTriggerInfo().size();
info.streams = streams_.GetStreamInfo().size();
return info;

View File

@ -302,7 +302,7 @@ class DbmsHandler {
auto db_acc_opt = db_gk.access();
if (db_acc_opt) {
auto &db_acc = *db_acc_opt;
const auto &info = db_acc->GetInfo(false, replication_role);
const auto &info = db_acc->GetInfo(replication_role);
const auto &storage_info = info.storage_info;
stats.num_vertex += storage_info.vertex_count;
stats.num_edges += storage_info.edge_count;
@ -338,7 +338,7 @@ class DbmsHandler {
auto db_acc_opt = db_gk.access();
if (db_acc_opt) {
auto &db_acc = *db_acc_opt;
res.push_back(db_acc->GetInfo(false, replication_role));
res.push_back(db_acc->GetInfo(replication_role));
}
}
return res;

View File

@ -825,7 +825,7 @@ uint64_t DiskStorage::GetDiskSpaceUsage() const {
durability_disk_storage_size;
}
StorageInfo DiskStorage::GetBaseInfo(bool /* unused */) {
StorageInfo DiskStorage::GetBaseInfo() {
StorageInfo info{};
info.vertex_count = vertex_count_;
info.edge_count = edge_count_.load(std::memory_order_acquire);
@ -838,9 +838,8 @@ StorageInfo DiskStorage::GetBaseInfo(bool /* unused */) {
return info;
}
StorageInfo DiskStorage::GetInfo(bool force_dir,
memgraph::replication_coordination_glue::ReplicationRole replication_role) {
StorageInfo info = GetBaseInfo(force_dir);
StorageInfo DiskStorage::GetInfo(memgraph::replication_coordination_glue::ReplicationRole replication_role) {
StorageInfo info = GetBaseInfo();
{
auto access = Access(replication_role);
const auto &lbl = access->ListAllIndices();

View File

@ -307,9 +307,8 @@ class DiskStorage final : public Storage {
std::vector<std::pair<std::string, std::string>> SerializeVerticesForLabelPropertyIndex(LabelId label,
PropertyId property);
StorageInfo GetBaseInfo(bool force_directory) override;
StorageInfo GetInfo(bool force_directory,
memgraph::replication_coordination_glue::ReplicationRole replication_role) override;
StorageInfo GetBaseInfo() override;
StorageInfo GetInfo(memgraph::replication_coordination_glue::ReplicationRole replication_role) override;
void FreeMemory(std::unique_lock<utils::ResourceLock> /*lock*/) override {}

View File

@ -11,6 +11,7 @@
#include "storage/v2/inmemory/storage.hpp"
#include <algorithm>
#include <filesystem>
#include <functional>
#include <optional>
#include "dbms/constants.hpp"
@ -1758,7 +1759,7 @@ void InMemoryStorage::CollectGarbage(std::unique_lock<utils::ResourceLock> main_
template void InMemoryStorage::CollectGarbage<true>(std::unique_lock<utils::ResourceLock>);
template void InMemoryStorage::CollectGarbage<false>(std::unique_lock<utils::ResourceLock>);
StorageInfo InMemoryStorage::GetBaseInfo(bool force_directory) {
StorageInfo InMemoryStorage::GetBaseInfo() {
StorageInfo info{};
info.vertex_count = vertices_.size();
info.edge_count = edge_count_.load(std::memory_order_acquire);
@ -1769,27 +1770,23 @@ StorageInfo InMemoryStorage::GetBaseInfo(bool force_directory) {
info.memory_res = utils::GetMemoryRES();
// Special case for the default database
auto update_path = [&](const std::filesystem::path &dir) {
if (!force_directory && std::filesystem::is_directory(dir) && dir.has_filename()) {
const auto end = dir.end();
auto it = end;
--it;
if (it != end) {
--it;
if (it != end && *it != "databases") {
// Default DB points to the root (for back-compatibility); update to the "database" dir
return dir / dbms::kMultiTenantDir / dbms::kDefaultDB;
}
#ifdef MG_ENTERPRISE
if (config_.salient.name == dbms::kDefaultDB) {
// Default DB points to the root (for back-compatibility); update to the "database" dir
std::filesystem::path new_dir = dir / "databases" / dbms::kDefaultDB;
if (std::filesystem::exists(new_dir) && std::filesystem::is_directory(new_dir)) {
return new_dir;
}
}
#endif
return dir;
};
info.disk_usage = utils::GetDirDiskUsage<false>(update_path(config_.durability.storage_directory));
return info;
}
StorageInfo InMemoryStorage::GetInfo(bool force_directory,
memgraph::replication_coordination_glue::ReplicationRole replication_role) {
StorageInfo info = GetBaseInfo(force_directory);
StorageInfo InMemoryStorage::GetInfo(memgraph::replication_coordination_glue::ReplicationRole replication_role) {
StorageInfo info = GetBaseInfo();
{
auto access = Access(replication_role); // TODO: override isolation level?
const auto &lbl = access->ListAllIndices();

View File

@ -368,9 +368,8 @@ class InMemoryStorage final : public Storage {
bool InitializeWalFile(memgraph::replication::ReplicationEpoch &epoch);
void FinalizeWalFile();
StorageInfo GetBaseInfo(bool force_directory) override;
StorageInfo GetInfo(bool force_directory,
memgraph::replication_coordination_glue::ReplicationRole replication_role) override;
StorageInfo GetBaseInfo() override;
StorageInfo GetInfo(memgraph::replication_coordination_glue::ReplicationRole replication_role) override;
/// Return true in all cases excepted if any sync replicas have not sent confirmation.
[[nodiscard]] bool AppendToWal(const Transaction &transaction, uint64_t final_commit_timestamp,

View File

@ -359,18 +359,9 @@ class Storage {
utils::BasicResult<SetIsolationLevelError> SetIsolationLevel(IsolationLevel isolation_level);
IsolationLevel GetIsolationLevel() const noexcept;
virtual StorageInfo GetBaseInfo(bool force_directory) = 0;
StorageInfo GetBaseInfo() {
#if MG_ENTERPRISE
const bool force_dir = false;
#else
const bool force_dir = true; //!< Use the configured directory (multi-tenancy reroutes to another dir)
#endif
return GetBaseInfo(force_dir);
}
virtual StorageInfo GetBaseInfo() = 0;
virtual StorageInfo GetInfo(bool force_directory,
memgraph::replication_coordination_glue::ReplicationRole replication_role) = 0;
virtual StorageInfo GetInfo(memgraph::replication_coordination_glue::ReplicationRole replication_role) = 0;
virtual Transaction CreateTransaction(IsolationLevel isolation_level, StorageMode storage_mode,
memgraph::replication_coordination_glue::ReplicationRole replication_role) = 0;

View File

@ -15,6 +15,7 @@
#include <optional>
#include "dbms/database.hpp"
#include "dbms/dbms_handler.hpp"
#include "disk_test_utils.hpp"
#include "query/interpret/awesome_memgraph_functions.hpp"
#include "query/interpreter_context.hpp"
@ -30,15 +31,31 @@ using namespace memgraph::storage;
constexpr auto testSuite = "database_v2_get_info";
const std::filesystem::path storage_directory{std::filesystem::temp_directory_path() / testSuite};
template <typename StorageType>
struct TestConfig {};
struct DefaultConfig : TestConfig {};
struct TenantConfig : TestConfig {};
template <typename TestType>
class InfoTest : public testing::Test {
using StorageType = typename TestType::first_type;
using ConfigType = typename TestType::second_type;
protected:
void SetUp() {
repl_state.emplace(memgraph::storage::ReplicationStateRootPath(config));
db_gk.emplace(config, *repl_state);
auto db_acc_opt = db_gk->access();
MG_ASSERT(db_acc_opt, "Failed to access db");
auto &db_acc = *db_acc_opt;
repl_state_.emplace(ReplicationStateRootPath(config));
#ifdef MG_ENTERPRISE
dbms_handler_.emplace(config, *repl_state_, auth_, false);
auto db_acc = dbms_handler_->Get(); // Default db
if (std::is_same_v<ConfigType, TenantConfig>) {
constexpr std::string_view db_name = "test_db";
MG_ASSERT(dbms_handler_->New(std::string{db_name}).HasValue(), "Failed to create database.");
db_acc = dbms_handler_->Get(db_name);
}
#else
dbms_handler_.emplace(config, *repl_state_);
auto db_acc = dbms_handler_->Get();
#endif
MG_ASSERT(db_acc, "Failed to access db");
MG_ASSERT(db_acc->GetStorageMode() == (std::is_same_v<StorageType, memgraph::storage::DiskStorage>
? memgraph::storage::StorageMode::ON_DISK_TRANSACTIONAL
: memgraph::storage::StorageMode::IN_MEMORY_TRANSACTIONAL),
@ -48,8 +65,8 @@ class InfoTest : public testing::Test {
void TearDown() {
db_acc_.reset();
db_gk.reset();
repl_state.reset();
dbms_handler_.reset();
repl_state_.reset();
if (std::is_same<StorageType, memgraph::storage::DiskStorage>::value) {
disk_test_utils::RemoveRocksDbDirs(testSuite);
}
@ -59,9 +76,9 @@ class InfoTest : public testing::Test {
StorageMode mode{std::is_same_v<StorageType, DiskStorage> ? StorageMode::ON_DISK_TRANSACTIONAL
: StorageMode::IN_MEMORY_TRANSACTIONAL};
std::optional<memgraph::replication::ReplicationState> repl_state;
std::optional<memgraph::dbms::DatabaseAccess> db_acc_;
std::optional<memgraph::utils::Gatekeeper<memgraph::dbms::Database>> db_gk;
#ifdef MG_ENTERPRISE
memgraph::auth::SynchedAuth auth_{storage_directory, memgraph::auth::Auth::Config {}};
#endif
memgraph::storage::Config config{
[&]() {
memgraph::storage::Config config{};
@ -69,18 +86,27 @@ class InfoTest : public testing::Test {
config.durability.snapshot_wal_mode =
memgraph::storage::Config::Durability::SnapshotWalMode::PERIODIC_SNAPSHOT_WITH_WAL;
if constexpr (std::is_same_v<StorageType, memgraph::storage::DiskStorage>) {
config.disk = disk_test_utils::GenerateOnDiskConfig(testSuite).disk;
config.force_on_disk = true;
}
return config;
}() // iile
};
std::optional<memgraph::replication::ReplicationState> repl_state_;
std::optional<memgraph::dbms::DbmsHandler> dbms_handler_;
std::optional<memgraph::dbms::DatabaseAccess> db_acc_;
};
using StorageTypes = ::testing::Types<memgraph::storage::InMemoryStorage, memgraph::storage::DiskStorage>;
using TestTypes = ::testing::Types<std::pair<memgraph::storage::InMemoryStorage, DefaultConfig>,
std::pair<memgraph::storage::DiskStorage, DefaultConfig>
TYPED_TEST_CASE(InfoTest, StorageTypes);
// TYPED_TEST_CASE(IndexTest, InMemoryStorageType);
#ifdef MG_ENTERPRISE
,
std::pair<memgraph::storage::InMemoryStorage, TenantConfig>,
std::pair<memgraph::storage::DiskStorage, TenantConfig>
#endif
>;
TYPED_TEST_CASE(InfoTest, TestTypes);
// NOLINTNEXTLINE(hicpp-special-member-functions)
TYPED_TEST(InfoTest, InfoCheck) {
@ -166,13 +192,13 @@ TYPED_TEST(InfoTest, InfoCheck) {
}
const auto &info = db_acc->GetInfo(
true, memgraph::replication_coordination_glue::ReplicationRole::MAIN); // force to use configured directory
memgraph::replication_coordination_glue::ReplicationRole::MAIN); // force to use configured directory
ASSERT_EQ(info.storage_info.vertex_count, 5);
ASSERT_EQ(info.storage_info.edge_count, 2);
ASSERT_EQ(info.storage_info.average_degree, 0.8);
ASSERT_GT(info.storage_info.memory_res, 10'000'000); // 200MB < > 10MB
ASSERT_LT(info.storage_info.memory_res, 200'000'000);
ASSERT_GT(info.storage_info.memory_res, 10'000'000); // 250MB < > 10MB
ASSERT_LT(info.storage_info.memory_res, 250'000'000);
ASSERT_GT(info.storage_info.disk_usage, 100); // 1MB < > 100B
ASSERT_LT(info.storage_info.disk_usage, 1000'000);
ASSERT_EQ(info.storage_info.label_indices, 1);

View File

@ -13,12 +13,11 @@
#include <gtest/gtest.h>
#include <filesystem>
#include "disk_test_utils.hpp"
#include "dbms/constants.hpp"
#include "storage/v2/disk/storage.hpp"
#include "storage/v2/inmemory/storage.hpp"
#include "storage/v2/isolation_level.hpp"
#include "storage/v2/storage.hpp"
#include "storage/v2/storage_error.hpp"
// NOLINTNEXTLINE(google-build-using-namespace)
using namespace memgraph::storage;
@ -31,6 +30,7 @@ class InfoTest : public testing::Test {
protected:
void SetUp() override {
std::filesystem::remove_all(storage_directory);
config_.salient.name = memgraph::dbms::kDefaultDB;
memgraph::storage::UpdatePaths(config_, storage_directory);
config_.durability.snapshot_wal_mode =
memgraph::storage::Config::Durability::SnapshotWalMode::PERIODIC_SNAPSHOT_WITH_WAL;
@ -135,7 +135,7 @@ TYPED_TEST(InfoTest, InfoCheck) {
ASSERT_FALSE(unique_acc->Commit().HasError());
}
StorageInfo info = this->storage->GetInfo(true, ReplicationRole::MAIN); // force to use configured directory
StorageInfo info = this->storage->GetInfo(ReplicationRole::MAIN);
ASSERT_EQ(info.vertex_count, 5);
ASSERT_EQ(info.edge_count, 2);