From a36753cf275066fb156251b283ea311bfe4b12c7 Mon Sep 17 00:00:00 2001
From: Dominik Gleich <dominik.gleich@memgraph.io>
Date: Wed, 4 Apr 2018 17:43:20 +0200
Subject: [PATCH] Update snapshot naming
Summary:
Snapshots should have the transaction from which they were created because we need this info for recovery later on.
Otherwise we wouldn't be able to tell the workers from which snapshots to recover. The whole cluster should be recovered
from the same transaction snapshot.
Reviewers: msantl
Reviewed By: msantl
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1338
---
src/durability/paths.cpp | 6 ++++--
src/durability/paths.hpp | 6 ++++--
src/durability/snapshooter.cpp | 7 ++++---
tests/manual/snapshot_generation/snapshot_writer.hpp | 4 +++-
tools/src/mg_import_csv/main.cpp | 4 +++-
5 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/durability/paths.cpp b/src/durability/paths.cpp
index ec851ee6a..a3254db3a 100644
--- a/src/durability/paths.cpp
+++ b/src/durability/paths.cpp
@@ -69,11 +69,13 @@ std::experimental::optional<tx::transaction_id_t> TransactionIdFromWalFilename(
}
}
-fs::path MakeSnapshotPath(const fs::path &durability_dir, const int worker_id) {
+fs::path MakeSnapshotPath(const fs::path &durability_dir, const int worker_id,
+ tx::transaction_id_t tx_id) {
std::string date_str =
Timestamp(Timestamp::Now())
.ToString("{:04d}_{:02d}_{:02d}__{:02d}_{:02d}_{:02d}_{:05d}");
- auto file_name = date_str + "_worker_" + std::to_string(worker_id);
+ auto file_name = date_str + "_worker_" + std::to_string(worker_id) + "_tx_" +
+ std::to_string(tx_id);
return durability_dir / kSnapshotDir / file_name;
}
diff --git a/src/durability/paths.hpp b/src/durability/paths.hpp
index 6790d84c4..7716016f0 100644
--- a/src/durability/paths.hpp
+++ b/src/durability/paths.hpp
@@ -26,9 +26,11 @@ std::experimental::optional<tx::transaction_id_t> TransactionIdFromWalFilename(
const std::string &name);
/** Generates a path for a DB snapshot in the given folder in a well-defined
- * sortable format with worker id appended to the file name. */
+ * sortable format with worker id and transaction from which the snapshot is
+ * created appended to the file name. */
std::experimental::filesystem::path MakeSnapshotPath(
- const std::experimental::filesystem::path &durability_dir, int worker_id);
+ const std::experimental::filesystem::path &durability_dir, int worker_id,
+ tx::transaction_id_t tx_id);
/// Generates a file path for a write-ahead log file of a specified worker. If
/// given a transaction ID the file name will contain it. Otherwise the file
diff --git a/src/durability/snapshooter.cpp b/src/durability/snapshooter.cpp
index f4ce972db..0327e8f8e 100644
--- a/src/durability/snapshooter.cpp
+++ b/src/durability/snapshooter.cpp
@@ -119,10 +119,11 @@ void RemoveOldWals(const fs::path &wal_dir,
bool MakeSnapshot(database::GraphDb &db, const fs::path &durability_dir,
const int snapshot_max_retained) {
- if (!EnsureDir(durability_dir / kSnapshotDir)) return false;
- const auto snapshot_file = MakeSnapshotPath(durability_dir, db.WorkerId());
- if (fs::exists(snapshot_file)) return false;
database::GraphDbAccessor dba(db);
+ if (!EnsureDir(durability_dir / kSnapshotDir)) return false;
+ const auto snapshot_file =
+ MakeSnapshotPath(durability_dir, db.WorkerId(), dba.transaction_id());
+ if (fs::exists(snapshot_file)) return false;
if (Encode(snapshot_file, db, dba)) {
RemoveOldSnapshots(durability_dir / kSnapshotDir, snapshot_max_retained);
RemoveOldWals(durability_dir / kWalDir, dba.transaction());
diff --git a/tests/manual/snapshot_generation/snapshot_writer.hpp b/tests/manual/snapshot_generation/snapshot_writer.hpp
index 579527ad0..62a3ea6a8 100644
--- a/tests/manual/snapshot_generation/snapshot_writer.hpp
+++ b/tests/manual/snapshot_generation/snapshot_writer.hpp
@@ -119,8 +119,10 @@ void WriteToSnapshot(GraphState &state, const std::string &path) {
LOG(ERROR) << "Unable to create durability directory!";
exit(0);
}
+ // Pretend like this is coming from transaction numbered 1 - this is hack
+ // TODO(dgleich): Change this to something more reasonable
const auto snapshot_file =
- durability::MakeSnapshotPath(durability_dir, worker_id);
+ durability::MakeSnapshotPath(durability_dir, worker_id, 1);
SnapshotWriter writer(snapshot_file, worker_id,
state.NumNodesOnWorker(worker_id),
diff --git a/tools/src/mg_import_csv/main.cpp b/tools/src/mg_import_csv/main.cpp
index 3d544dea5..47a7a3339 100644
--- a/tools/src/mg_import_csv/main.cpp
+++ b/tools/src/mg_import_csv/main.cpp
@@ -470,7 +470,9 @@ std::string GetOutputPath() {
LOG(FATAL) << error.what();
}
int worker_id = 0;
- return std::string(durability::MakeSnapshotPath(durability_dir, worker_id));
+ // TODO(dgleich): Remove this transaction id hack
+ return std::string(
+ durability::MakeSnapshotPath(durability_dir, worker_id, 1));
}
int main(int argc, char *argv[]) {