Move creating snapshot from constructor to Open()

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();

src/storage/storage.cc

@@ -76,14 +76,19 @@ 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/)
   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; });
 
+  // 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<rocksdb::Checkpoint> 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<DBStats>()) {
-  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;
 

src/storage/storage.h

@@ -28,7 +28,6 @@
 #include <rocksdb/utilities/write_batch_with_index.h>
 
 #include <atomic>
-#include <cinttypes>
 #include <cstddef>
 #include <memory>
 #include <shared_mutex>