From e8fd9919768fdde0ccd98d779f3ceda08d9f3f6e Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Fri, 20 Sep 2024 14:24:17 -0400 Subject: [PATCH 01/11] Add checkpoints --- src/cli/main.cc | 55 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/cli/main.cc b/src/cli/main.cc index a6ed2025873..112b16d59ea 100644 --- a/src/cli/main.cc +++ b/src/cli/main.cc @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -34,6 +35,7 @@ #include "daemon_util.h" #include "io_util.h" #include "pid_util.h" +#include "rocksdb/utilities/checkpoint.h" #include "scope_exit.h" #include "server/server.h" #include "signal_util.h" @@ -46,6 +48,8 @@ #include "version_util.h" Server *srv = nullptr; +std::optional checkpoint_location = std::nullopt; +bool keep_checkpoint = false; extern "C" void SignalHandler([[maybe_unused]] int sig) { if (srv && !srv->IsStopped()) { @@ -67,6 +71,10 @@ static void PrintUsage(const char *program) { << "print version information" << std::endl << new_opt << "-h, --help" << "print this help message" << std::endl + << new_opt << "--use-checkpoint " + << "create a checkpoint of the supplied database in and start a read-only server using that checkpoint" << std::endl + << new_opt << "--keep-checkpoint" + << "keep the checkpoint after the server is stopped" << std::endl << new_opt << "-- " << "overwrite specific config option to " << std::endl; } @@ -84,6 +92,10 @@ static CLIOptions ParseCommandLineOptions(int argc, char **argv) { } else if (argv[i] == "-h"sv || argv[i] == "--help"sv) { PrintUsage(*argv); std::exit(0); + } else if (argv[i] == "--use-checkpoint"sv && i + 1 < argc) { + checkpoint_location = argv[++i]; + } else if (argv[i] == "--keep-checkpoint"sv) { + keep_checkpoint = true; } else if (std::string_view(argv[i], 2) == "--" && std::string_view(argv[i]).size() > 2 && i + 1 < argc) { auto key = std::string_view(argv[i] + 2); opts.cli_options.emplace_back(key, argv[++i]); @@ -116,6 +128,32 @@ static void InitGoogleLog(const Config *config) { } } +static void CreateCheckpoint(Config &config, const std::string &checkpoint_location) { + // The Storage destructor deletes anything at the checkpoint_dir, so we need to make sure it's empty in case the user + // happens to use a checkpoint folder name which matches the default (checkpoint/) + const std::string old_checkpoint_dir = std::exchange(config.checkpoint_dir, ""); + const auto checkpoint_dir_guard = + MakeScopeExit([&config, &old_checkpoint_dir] { config.checkpoint_dir = old_checkpoint_dir; }); + + engine::Storage storage(&config); + if (const auto s = storage.Open(kDBOpenModeForReadOnly); !s.IsOK()) { + LOG(ERROR) << "Failed to open: " << s.Msg(); + throw std::runtime_error("Failed to open DB in read-only mode"); + } + + rocksdb::Checkpoint *checkpoint = nullptr; + if (const auto s = rocksdb::Checkpoint::Create(storage.GetDB(), &checkpoint); !s.ok()) { + LOG(ERROR) << "Failed to create checkpoint: " << s.ToString(); + throw std::runtime_error("Failed to create checkpoint"); + } + + std::unique_ptr checkpoint_guard(checkpoint); + if (const auto s = checkpoint->CreateCheckpoint(checkpoint_location); !s.ok()) { + LOG(ERROR) << "Failed to create checkpoint: " << s.ToString(); + throw std::runtime_error("Failed to create checkpoint"); + } +} + int main(int argc, char *argv[]) { srand(static_cast(util::GetTimeStamp())); @@ -168,8 +206,23 @@ int main(int argc, char *argv[]) { } #endif + const bool read_only = checkpoint_location.has_value(); + if (read_only) { + CreateCheckpoint(config, checkpoint_location.value()); + LOG(INFO) << "Starting server in read-only mode with checkpoint dir: " << checkpoint_location.value(); + config.db_dir = checkpoint_location.value(); + } + const auto checkpoint_exit = MakeScopeExit([]() { + if (!keep_checkpoint && checkpoint_location.has_value()) { + LOG(INFO) << "Removing checkpoint dir: " << checkpoint_location.value(); + std::filesystem::remove_all(checkpoint_location.value()); + } + }); + engine::Storage storage(&config); - s = storage.Open(); + const DBOpenMode open_mode = read_only ? kDBOpenModeForReadOnly : kDBOpenModeDefault; + s = storage.Open(open_mode); + if (!s.IsOK()) { LOG(ERROR) << "Failed to open: " << s.Msg(); return 1; From b2ce62b2060c3a1caecd679119512ce79e6e8b3f Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Tue, 24 Sep 2024 15:54:41 -0400 Subject: [PATCH 02/11] Move global variables to Config --- src/cli/main.cc | 47 +++++++++++++++++++------------------------- src/config/config.cc | 2 ++ src/config/config.h | 5 +++++ 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/cli/main.cc b/src/cli/main.cc index 112b16d59ea..d38b935f801 100644 --- a/src/cli/main.cc +++ b/src/cli/main.cc @@ -48,8 +48,6 @@ #include "version_util.h" Server *srv = nullptr; -std::optional checkpoint_location = std::nullopt; -bool keep_checkpoint = false; extern "C" void SignalHandler([[maybe_unused]] int sig) { if (srv && !srv->IsStopped()) { @@ -71,10 +69,6 @@ static void PrintUsage(const char *program) { << "print version information" << std::endl << new_opt << "-h, --help" << "print this help message" << std::endl - << new_opt << "--use-checkpoint " - << "create a checkpoint of the supplied database in and start a read-only server using that checkpoint" << std::endl - << new_opt << "--keep-checkpoint" - << "keep the checkpoint after the server is stopped" << std::endl << new_opt << "-- " << "overwrite specific config option to " << std::endl; } @@ -92,10 +86,6 @@ static CLIOptions ParseCommandLineOptions(int argc, char **argv) { } else if (argv[i] == "-h"sv || argv[i] == "--help"sv) { PrintUsage(*argv); std::exit(0); - } else if (argv[i] == "--use-checkpoint"sv && i + 1 < argc) { - checkpoint_location = argv[++i]; - } else if (argv[i] == "--keep-checkpoint"sv) { - keep_checkpoint = true; } else if (std::string_view(argv[i], 2) == "--" && std::string_view(argv[i]).size() > 2 && i + 1 < argc) { auto key = std::string_view(argv[i] + 2); opts.cli_options.emplace_back(key, argv[++i]); @@ -128,7 +118,7 @@ static void InitGoogleLog(const Config *config) { } } -static void CreateCheckpoint(Config &config, const std::string &checkpoint_location) { +static Status CreateCheckpoint(Config &config, const std::string &checkpoint_location) { // The Storage destructor deletes anything at the checkpoint_dir, so we need to make sure it's empty in case the user // happens to use a checkpoint folder name which matches the default (checkpoint/) const std::string old_checkpoint_dir = std::exchange(config.checkpoint_dir, ""); @@ -137,21 +127,20 @@ static void CreateCheckpoint(Config &config, const std::string &checkpoint_locat engine::Storage storage(&config); if (const auto s = storage.Open(kDBOpenModeForReadOnly); !s.IsOK()) { - LOG(ERROR) << "Failed to open: " << s.Msg(); - throw std::runtime_error("Failed to open DB in read-only mode"); + return {Status::NotOK, fmt::format("failed to open DB in read-only mode: {}", s.Msg())}; } rocksdb::Checkpoint *checkpoint = nullptr; if (const auto s = rocksdb::Checkpoint::Create(storage.GetDB(), &checkpoint); !s.ok()) { - LOG(ERROR) << "Failed to create checkpoint: " << s.ToString(); - throw std::runtime_error("Failed to create checkpoint"); + return {Status::NotOK, fmt::format("failed to create checkpoint: {}", s.ToString())}; } std::unique_ptr checkpoint_guard(checkpoint); - if (const auto s = checkpoint->CreateCheckpoint(checkpoint_location); !s.ok()) { - LOG(ERROR) << "Failed to create checkpoint: " << s.ToString(); - throw std::runtime_error("Failed to create checkpoint"); + if (const auto s = checkpoint->CreateCheckpoint(checkpoint_location + "/db"); !s.ok()) { + return {Status::NotOK, fmt::format("failed to create checkpoint: {}", s.ToString())}; } + + return Status::OK(); } int main(int argc, char *argv[]) { @@ -206,20 +195,24 @@ int main(int argc, char *argv[]) { } #endif - const bool read_only = checkpoint_location.has_value(); - if (read_only) { - CreateCheckpoint(config, checkpoint_location.value()); - LOG(INFO) << "Starting server in read-only mode with checkpoint dir: " << checkpoint_location.value(); - config.db_dir = checkpoint_location.value(); + const bool use_snapshot = config.snapshot_dir != ""; + if (use_snapshot) { + if (const auto s = CreateCheckpoint(config, config.snapshot_dir); !s.IsOK()) { + LOG(ERROR) << "Failed to create checkpoint: " << s.Msg(); + return 1; + } + LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config.snapshot_dir; + config.db_dir = config.snapshot_dir + "/db"; } - const auto checkpoint_exit = MakeScopeExit([]() { - if (!keep_checkpoint && checkpoint_location.has_value()) { - LOG(INFO) << "Removing checkpoint dir: " << checkpoint_location.value(); - std::filesystem::remove_all(checkpoint_location.value()); + const auto checkpoint_exit = MakeScopeExit([use_snapshot, &config]() { + if (use_snapshot && !config.keep_checkpoint) { + LOG(INFO) << "Removing snapshot dir: " << config.snapshot_dir; + std::filesystem::remove_all(config.snapshot_dir); } }); engine::Storage storage(&config); + const bool read_only = use_snapshot; const DBOpenMode open_mode = read_only ? kDBOpenModeForReadOnly : kDBOpenModeDefault; s = storage.Open(open_mode); diff --git a/src/config/config.cc b/src/config/config.cc index 1abdb913cb7..287797d0e09 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -148,6 +148,8 @@ Config::Config() { new IntField(&force_compact_file_min_deleted_percentage, 10, 1, 100)}, {"db-name", true, new StringField(&db_name, "change.me.db")}, {"dir", true, new StringField(&dir, kDefaultDir)}, + {"snapshot-dir", false, new StringField(&snapshot_dir, "")}, + {"keep-snapshot", false, new YesNoField(&keep_snapshot, false)}, {"backup-dir", false, new StringField(&backup_dir, kDefaultBackupDir)}, {"log-dir", true, new StringField(&log_dir, "")}, {"log-level", false, new EnumField(&log_level, log_levels, google::INFO)}, diff --git a/src/config/config.h b/src/config/config.h index 8a2ebfc4786..e80e8578776 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -138,6 +138,11 @@ struct Config { std::string replica_announce_ip; uint32_t replica_announce_port = 0; + // The following options exist so users can spawn a read-only server from a snapshot + // of a running server. + std::string snapshot_dir; + bool keep_snapshot = false; + bool persist_cluster_nodes_enabled = true; bool slot_id_encoded = false; bool cluster_enabled = false; From b3e93fe3c16476dc7191f6b95892359f2ce0736d Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Sun, 29 Sep 2024 22:00:12 -0400 Subject: [PATCH 03/11] Fix lint error, rename checkpoint -> snapshot, deduplicate error messages --- src/cli/main.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cli/main.cc b/src/cli/main.cc index d38b935f801..1dd52bc9266 100644 --- a/src/cli/main.cc +++ b/src/cli/main.cc @@ -118,9 +118,9 @@ static void InitGoogleLog(const Config *config) { } } -static Status CreateCheckpoint(Config &config, const std::string &checkpoint_location) { +static Status CreateSnapshot(Config &config, const std::string &snapshot_location) { // The Storage destructor deletes anything at the checkpoint_dir, so we need to make sure it's empty in case the user - // happens to use a checkpoint folder name which matches the default (checkpoint/) + // happens to use a snapshot name which matches the default (checkpoint/) const std::string old_checkpoint_dir = std::exchange(config.checkpoint_dir, ""); const auto checkpoint_dir_guard = MakeScopeExit([&config, &old_checkpoint_dir] { config.checkpoint_dir = old_checkpoint_dir; }); @@ -130,14 +130,14 @@ static Status CreateCheckpoint(Config &config, const std::string &checkpoint_loc return {Status::NotOK, fmt::format("failed to open DB in read-only mode: {}", s.Msg())}; } - rocksdb::Checkpoint *checkpoint = nullptr; - if (const auto s = rocksdb::Checkpoint::Create(storage.GetDB(), &checkpoint); !s.ok()) { - return {Status::NotOK, fmt::format("failed to create checkpoint: {}", s.ToString())}; + rocksdb::Checkpoint *snapshot = nullptr; + if (const auto s = rocksdb::Checkpoint::Create(storage.GetDB(), &snapshot); !s.ok()) { + return {Status::NotOK, s.ToString()}; } - std::unique_ptr checkpoint_guard(checkpoint); - if (const auto s = checkpoint->CreateCheckpoint(checkpoint_location + "/db"); !s.ok()) { - return {Status::NotOK, fmt::format("failed to create checkpoint: {}", s.ToString())}; + std::unique_ptr snapshot_guard(snapshot); + if (const auto s = snapshot->CreateCheckpoint(snapshot_location + "/db"); !s.ok()) { + return {Status::NotOK, s.ToString()}; } return Status::OK(); @@ -197,15 +197,15 @@ int main(int argc, char *argv[]) { const bool use_snapshot = config.snapshot_dir != ""; if (use_snapshot) { - if (const auto s = CreateCheckpoint(config, config.snapshot_dir); !s.IsOK()) { - LOG(ERROR) << "Failed to create checkpoint: " << s.Msg(); + if (const auto s = CreateSnapshot(config, config.snapshot_dir); !s.IsOK()) { + LOG(ERROR) << "Failed to create snapshot: " << s.Msg(); return 1; } LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config.snapshot_dir; config.db_dir = config.snapshot_dir + "/db"; } - const auto checkpoint_exit = MakeScopeExit([use_snapshot, &config]() { - if (use_snapshot && !config.keep_checkpoint) { + const auto snapshot_exit = MakeScopeExit([use_snapshot, &config]() { + if (use_snapshot && !config.keep_snapshot) { LOG(INFO) << "Removing snapshot dir: " << config.snapshot_dir; std::filesystem::remove_all(config.snapshot_dir); } From 810a86c80749e4c8e2f95bf06f87a69a2c5706b6 Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Thu, 3 Oct 2024 17:03:28 -0400 Subject: [PATCH 04/11] Trigger tests From d27c1bce252179660a246d9ef518929604771f9d Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Mon, 21 Oct 2024 15:28:27 -0400 Subject: [PATCH 05/11] Move functionality out of main.cc --- kvrocks.conf | 7 +++++++ src/cli/main.cc | 45 +----------------------------------------- src/config/config.cc | 3 --- src/config/config.h | 3 +-- src/storage/storage.cc | 35 ++++++++++++++++++++++++++++++++ 5 files changed, 44 insertions(+), 49 deletions(-) diff --git a/kvrocks.conf b/kvrocks.conf index e7de065c5ea..b97175061c5 100644 --- a/kvrocks.conf +++ b/kvrocks.conf @@ -127,6 +127,13 @@ db-name change.me.db # Note that you must specify a directory here, not a file name. dir /tmp/kvrocks +# Optional: A snapshot directory +# +# If specified, Kvrocks will snapshot the DB at [dir] to the specified [snapshot-dir], +# and start a read-only server from the snapshot. +# +# snapshot-dir /tmp/kvrocks-snapshot-changeme + # You can configure where to store your server logs by the log-dir. # If you don't specify one, we will use the above `dir` as our default log directory. # We also can send logs to stdout/stderr is as simple as: diff --git a/src/cli/main.cc b/src/cli/main.cc index 4008d5cc18e..82ff477d04b 100644 --- a/src/cli/main.cc +++ b/src/cli/main.cc @@ -27,14 +27,12 @@ #include #include -#include #include #include #include "daemon_util.h" #include "io_util.h" #include "pid_util.h" -#include "rocksdb/utilities/checkpoint.h" #include "scope_exit.h" #include "server/server.h" #include "signal_util.h" @@ -115,31 +113,6 @@ static void InitGoogleLog(const Config *config) { } } -static Status CreateSnapshot(Config &config, const std::string &snapshot_location) { - // The Storage destructor deletes anything at the checkpoint_dir, so we need to make sure it's empty in case the user - // happens to use a snapshot name which matches the default (checkpoint/) - const std::string old_checkpoint_dir = std::exchange(config.checkpoint_dir, ""); - const auto checkpoint_dir_guard = - MakeScopeExit([&config, &old_checkpoint_dir] { config.checkpoint_dir = old_checkpoint_dir; }); - - engine::Storage storage(&config); - if (const auto s = storage.Open(kDBOpenModeForReadOnly); !s.IsOK()) { - return {Status::NotOK, fmt::format("failed to open DB in read-only mode: {}", s.Msg())}; - } - - rocksdb::Checkpoint *snapshot = nullptr; - if (const auto s = rocksdb::Checkpoint::Create(storage.GetDB(), &snapshot); !s.ok()) { - return {Status::NotOK, s.ToString()}; - } - - std::unique_ptr snapshot_guard(snapshot); - if (const auto s = snapshot->CreateCheckpoint(snapshot_location + "/db"); !s.ok()) { - return {Status::NotOK, s.ToString()}; - } - - return Status::OK(); -} - int main(int argc, char *argv[]) { srand(static_cast(util::GetTimeStamp())); @@ -197,24 +170,8 @@ int main(int argc, char *argv[]) { } #endif - const bool use_snapshot = config.snapshot_dir != ""; - if (use_snapshot) { - if (const auto s = CreateSnapshot(config, config.snapshot_dir); !s.IsOK()) { - LOG(ERROR) << "Failed to create snapshot: " << s.Msg(); - return 1; - } - LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config.snapshot_dir; - config.db_dir = config.snapshot_dir + "/db"; - } - const auto snapshot_exit = MakeScopeExit([use_snapshot, &config]() { - if (use_snapshot && !config.keep_snapshot) { - LOG(INFO) << "Removing snapshot dir: " << config.snapshot_dir; - std::filesystem::remove_all(config.snapshot_dir); - } - }); - engine::Storage storage(&config); - const bool read_only = use_snapshot; + const bool read_only = config.snapshot_dir != ""; const DBOpenMode open_mode = read_only ? kDBOpenModeForReadOnly : kDBOpenModeDefault; s = storage.Open(open_mode); diff --git a/src/config/config.cc b/src/config/config.cc index 9c972057d97..a4813498526 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -24,13 +24,11 @@ #include #include -#include #include #include #include #include #include -#include #include #include #include @@ -191,7 +189,6 @@ Config::Config() { {"db-name", true, new StringField(&db_name, "change.me.db")}, {"dir", true, new StringField(&dir, kDefaultDir)}, {"snapshot-dir", false, new StringField(&snapshot_dir, "")}, - {"keep-snapshot", false, new YesNoField(&keep_snapshot, false)}, {"backup-dir", false, new StringField(&backup_dir, kDefaultBackupDir)}, {"log-dir", true, new StringField(&log_dir, "")}, {"log-level", false, new EnumField(&log_level, log_levels, google::INFO)}, diff --git a/src/config/config.h b/src/config/config.h index faecd2bb14f..9343d755df4 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -140,10 +140,9 @@ struct Config { std::string replica_announce_ip; uint32_t replica_announce_port = 0; - // The following options exist so users can spawn a read-only server from a snapshot + // The following option exists so users can spawn a read-only server from a snapshot // of a running server. std::string snapshot_dir; - bool keep_snapshot = false; bool persist_cluster_nodes_enabled = true; bool slot_id_encoded = false; diff --git a/src/storage/storage.cc b/src/storage/storage.cc index 2eead08ace6..c4c574850cb 100644 --- a/src/storage/storage.cc +++ b/src/storage/storage.cc @@ -44,6 +44,7 @@ #include "redis_metadata.h" #include "rocksdb/cache.h" #include "rocksdb_crc32c.h" +#include "scope_exit.h" #include "server/server.h" #include "storage/batch_indexer.h" #include "table_properties_collector.h" @@ -75,12 +76,46 @@ const int64_t kIORateLimitMaxMb = 1024000; using rocksdb::Slice; +static Status CreateSnapshot(Config &config, const std::string &snapshot_location) { + // The Storage destructor deletes anything at the checkpoint_dir, so we need to make + // sure it's empty in case the user happens to use a snapshot name which matches the + // default (checkpoint/) + const std::string old_checkpoint_dir = std::exchange(config.checkpoint_dir, ""); + const auto checkpoint_dir_guard = + MakeScopeExit([&config, &old_checkpoint_dir] { config.checkpoint_dir = old_checkpoint_dir; }); + + engine::Storage storage(&config); + if (const auto s = storage.Open(kDBOpenModeForReadOnly); !s.IsOK()) { + return {Status::NotOK, fmt::format("failed to open DB in read-only mode: {}", s.Msg())}; + } + + rocksdb::Checkpoint *snapshot = nullptr; + if (const auto s = rocksdb::Checkpoint::Create(storage.GetDB(), &snapshot); !s.ok()) { + return {Status::NotOK, s.ToString()}; + } + + std::unique_ptr snapshot_guard(snapshot); + if (const auto s = snapshot->CreateCheckpoint(snapshot_location + "/db"); !s.ok()) { + return {Status::NotOK, s.ToString()}; + } + + return Status::OK(); +} + Storage::Storage(Config *config) : backup_creating_time_secs_(util::GetTimeStamp()), env_(rocksdb::Env::Default()), config_(config), lock_mgr_(16), db_stats_(std::make_unique()) { + if (config->snapshot_dir != "") { + if (const auto s = CreateSnapshot(*config, config->snapshot_dir); !s.IsOK()) { + throw std::runtime_error(fmt::format("Failed to create snapshot: {}", s.Msg())); + } + LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config->snapshot_dir; + config->db_dir = config->snapshot_dir + "/db"; + } + Metadata::InitVersionCounter(); SetWriteOptions(config->rocks_db.write_options); } From e46818ade794506f08681ea3f1309512c5265b3a Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Thu, 24 Oct 2024 12:31:16 -0400 Subject: [PATCH 06/11] Format --- src/storage/storage.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage.cc b/src/storage/storage.cc index c4c574850cb..8e85b20a00c 100644 --- a/src/storage/storage.cc +++ b/src/storage/storage.cc @@ -77,8 +77,8 @@ const int64_t kIORateLimitMaxMb = 1024000; using rocksdb::Slice; static Status CreateSnapshot(Config &config, const std::string &snapshot_location) { - // The Storage destructor deletes anything at the checkpoint_dir, so we need to make - // sure it's empty in case the user happens to use a snapshot name which matches the + // The Storage destructor deletes anything at the checkpoint_dir, so we need to make + // sure it's empty in case the user happens to use a snapshot name which matches the // default (checkpoint/) const std::string old_checkpoint_dir = std::exchange(config.checkpoint_dir, ""); const auto checkpoint_dir_guard = From 53b0ed7f8caa118fb71b0e875ef52160d6fc634d Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Tue, 5 Nov 2024 18:05:14 -0500 Subject: [PATCH 07/11] Move creating snapshot from constructor to Open() --- src/cli/main.cc | 4 +--- src/storage/storage.cc | 29 +++++++++++++++++++---------- src/storage/storage.h | 1 - 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/cli/main.cc b/src/cli/main.cc index 82ff477d04b..defb4d12f3f 100644 --- a/src/cli/main.cc +++ b/src/cli/main.cc @@ -171,9 +171,7 @@ int main(int argc, char *argv[]) { #endif engine::Storage storage(&config); - const bool read_only = config.snapshot_dir != ""; - const DBOpenMode open_mode = read_only ? kDBOpenModeForReadOnly : kDBOpenModeDefault; - s = storage.Open(open_mode); + s = storage.Open(kDBOpenModeDefault); if (!s.IsOK()) { LOG(ERROR) << "Failed to open: " << s.Msg(); diff --git a/src/storage/storage.cc b/src/storage/storage.cc index 6292e44993c..5ef99c8c19a 100644 --- a/src/storage/storage.cc +++ b/src/storage/storage.cc @@ -76,7 +76,7 @@ const int64_t kIORateLimitMaxMb = 1024000; using rocksdb::Slice; -static Status CreateSnapshot(Config &config, const std::string &snapshot_location) { +static Status CreateSnapshot(Config &config) { // The Storage destructor deletes anything at the checkpoint_dir, so we need to make // sure it's empty in case the user happens to use a snapshot name which matches the // default (checkpoint/) @@ -84,6 +84,11 @@ static Status CreateSnapshot(Config &config, const std::string &snapshot_locatio const auto checkpoint_dir_guard = MakeScopeExit([&config, &old_checkpoint_dir] { config.checkpoint_dir = old_checkpoint_dir; }); + // Since .Open() will call `CreateSnapshot` if `snapshot_dir` is set, we need to + // clear it, and reset it after the snapshot is created to preserve symmetry. + const std::string snapshot_dir = std::exchange(config.snapshot_dir, ""); + const auto snapshot_dir_guard = MakeScopeExit([&config, &snapshot_dir] { config.snapshot_dir = snapshot_dir; }); + engine::Storage storage(&config); if (const auto s = storage.Open(kDBOpenModeForReadOnly); !s.IsOK()) { return {Status::NotOK, fmt::format("failed to open DB in read-only mode: {}", s.Msg())}; @@ -95,7 +100,7 @@ static Status CreateSnapshot(Config &config, const std::string &snapshot_locatio } std::unique_ptr snapshot_guard(snapshot); - if (const auto s = snapshot->CreateCheckpoint(snapshot_location + "/db"); !s.ok()) { + if (const auto s = snapshot->CreateCheckpoint(snapshot_dir + "/db"); !s.ok()) { return {Status::NotOK, s.ToString()}; } @@ -108,14 +113,6 @@ Storage::Storage(Config *config) config_(config), lock_mgr_(16), db_stats_(std::make_unique()) { - if (config->snapshot_dir != "") { - if (const auto s = CreateSnapshot(*config, config->snapshot_dir); !s.IsOK()) { - throw std::runtime_error(fmt::format("Failed to create snapshot: {}", s.Msg())); - } - LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config->snapshot_dir; - config->db_dir = config->snapshot_dir + "/db"; - } - Metadata::InitVersionCounter(); SetWriteOptions(config->rocks_db.write_options); } @@ -302,6 +299,18 @@ Status Storage::CreateColumnFamilies(const rocksdb::Options &options) { } Status Storage::Open(DBOpenMode mode) { + // If a snapshot directory was specified, create a snapshot and open the database + // there in read-only mode. + if (config_->snapshot_dir != "") { + if (const auto s = CreateSnapshot(*config_); !s.IsOK()) { + return s; + } + LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config_->snapshot_dir; + config_->dir = config_->snapshot_dir; + config_->db_dir = config_->snapshot_dir + "/db"; + mode = DBOpenMode::kDBOpenModeForReadOnly; + } + auto guard = WriteLockGuard(); db_closing_ = false; diff --git a/src/storage/storage.h b/src/storage/storage.h index c29b4689de5..dce5617949f 100644 --- a/src/storage/storage.h +++ b/src/storage/storage.h @@ -28,7 +28,6 @@ #include #include -#include #include #include #include From 7f6d0a22a82decefdcee7a767b59c77b1a798dbb Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Tue, 5 Nov 2024 18:08:58 -0500 Subject: [PATCH 08/11] Make snapshot-dir read-only --- src/config/config.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.cc b/src/config/config.cc index f1486f88df4..86073a55f78 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -188,7 +188,7 @@ Config::Config() { new IntField(&force_compact_file_min_deleted_percentage, 10, 1, 100)}, {"db-name", true, new StringField(&db_name, "change.me.db")}, {"dir", true, new StringField(&dir, kDefaultDir)}, - {"snapshot-dir", false, new StringField(&snapshot_dir, "")}, + {"snapshot-dir", true, new StringField(&snapshot_dir, "")}, {"backup-dir", false, new StringField(&backup_dir, kDefaultBackupDir)}, {"log-dir", true, new StringField(&log_dir, "")}, {"log-level", false, new EnumField(&log_level, log_levels, google::INFO)}, From 694f83912331eb69ef687e747bf6d68db9dc7119 Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Wed, 6 Nov 2024 10:16:35 -0500 Subject: [PATCH 09/11] Remove trailing whitespace --- src/storage/storage.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage.cc b/src/storage/storage.cc index 5ef99c8c19a..b58abc20545 100644 --- a/src/storage/storage.cc +++ b/src/storage/storage.cc @@ -84,7 +84,7 @@ static Status CreateSnapshot(Config &config) { const auto checkpoint_dir_guard = MakeScopeExit([&config, &old_checkpoint_dir] { config.checkpoint_dir = old_checkpoint_dir; }); - // Since .Open() will call `CreateSnapshot` if `snapshot_dir` is set, we need to + // Since .Open() will call `CreateSnapshot` if `snapshot_dir` is set, we need to // clear it, and reset it after the snapshot is created to preserve symmetry. const std::string snapshot_dir = std::exchange(config.snapshot_dir, ""); const auto snapshot_dir_guard = MakeScopeExit([&config, &snapshot_dir] { config.snapshot_dir = snapshot_dir; }); From 19f25b9dc2ade3efab96b1142aff61a53d1acba5 Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Wed, 6 Nov 2024 11:38:10 -0500 Subject: [PATCH 10/11] Fix 'const prevents move' issue --- src/storage/storage.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage.cc b/src/storage/storage.cc index b58abc20545..635c8fbf222 100644 --- a/src/storage/storage.cc +++ b/src/storage/storage.cc @@ -302,7 +302,7 @@ Status Storage::Open(DBOpenMode mode) { // If a snapshot directory was specified, create a snapshot and open the database // there in read-only mode. if (config_->snapshot_dir != "") { - if (const auto s = CreateSnapshot(*config_); !s.IsOK()) { + if (auto s = CreateSnapshot(*config_); !s.IsOK()) { return s; } LOG(INFO) << "Starting server in read-only mode with snapshot dir: " << config_->snapshot_dir; From 786908ff5ab8db257e27029b915938d00d6e9872 Mon Sep 17 00:00:00 2001 From: Nathan Lo Date: Thu, 7 Nov 2024 16:32:17 -0500 Subject: [PATCH 11/11] Revert changes in main.cc --- src/cli/main.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/main.cc b/src/cli/main.cc index defb4d12f3f..3eaf4b13b6b 100644 --- a/src/cli/main.cc +++ b/src/cli/main.cc @@ -171,8 +171,7 @@ int main(int argc, char *argv[]) { #endif engine::Storage storage(&config); - s = storage.Open(kDBOpenModeDefault); - + s = storage.Open(); if (!s.IsOK()) { LOG(ERROR) << "Failed to open: " << s.Msg(); return 1;