Skip to content

Commit

Permalink
change error message at persistent format version check
Browse files Browse the repository at this point in the history
  • Loading branch information
@ban-nobuhiro
ban-nobuhiro committed Dec 6, 2023
1 parent 4a86c17 commit 4847e6b
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 22 deletions.
29 changes: 17 additions & 12 deletions src/limestone/datastore_format.cpp
View file
Expand Up @@ -61,48 +61,53 @@ void setup_initial_logdir(const boost::filesystem::path& logdir) {
}
}

static constexpr const char *version_error_prefix = "/:limestone unsupported dbdir format version: "
static constexpr const char *version_error_prefix = "/:limestone unsupported dbdir persistent format version: "
"see https://github.com/project-tsurugi/tsurugidb/blob/master/docs/upgrade-guide.md";

bool is_supported_version(const boost::filesystem::path& manifest_path, std::string& errmsg) {
int is_supported_version(const boost::filesystem::path& manifest_path, std::string& errmsg) {
std::ifstream istrm(manifest_path.string());
if (!istrm) {
errmsg = "cannot open for read " + manifest_path.string();
return false;
return 0;
}
nlohmann::json manifest;
try {
istrm >> manifest;
auto version = manifest["persistent_format_version"];
if (version.is_number_integer()) {
if (version == 1) {
return true; // supported
return 1; // supported
}
errmsg = "format version mismatch: version " + version.dump();
return false;
errmsg = "version mismatch: version " + version.dump() + ", server supports version 1";
return 0;
}
errmsg = "invalid manifest file, invalid persistent_format_version: " + version.dump();
return false;
return -1;
} catch (nlohmann::json::exception& e) {
errmsg = "invalid manifest file, JSON parse error: ";
errmsg.append(e.what());
return false;
return -1;
};
return true;
}

void check_logdir_format(const boost::filesystem::path& logdir) {
boost::filesystem::path manifest_path = logdir / std::string(manifest_file_name);
if (!boost::filesystem::exists(manifest_path)) {
LOG_LP(INFO) << "no manifest file in logdir, maybe v0";
LOG(ERROR) << version_error_prefix << " (format version mismatch: version 0)";
VLOG_LP(log_info) << "no manifest file in logdir, maybe v0";
LOG(ERROR) << version_error_prefix << " (version mismatch: version 0, server supports version 1)";
throw std::runtime_error("logdir version mismatch");
}
std::string errmsg;
if (!is_supported_version(manifest_path, errmsg)) {
int vc = is_supported_version(manifest_path, errmsg);
if (vc == 0) {
LOG(ERROR) << version_error_prefix << " (" << errmsg << ")";
throw std::runtime_error("logdir version mismatch");
}
if (vc < 0) {
VLOG_LP(log_info) << errmsg;
LOG(ERROR) << "/:limestone dbdir is corrupted, can not use.";
throw std::runtime_error("logdir corrupted");
}
}

} // namespace limestone::internal
26 changes: 19 additions & 7 deletions src/limestone/datastore_restore.cpp
View file
Expand Up @@ -26,7 +26,7 @@

namespace limestone::api {

static constexpr const char *version_error_prefix = "/:limestone unsupported backup format version: "
static constexpr const char *version_error_prefix = "/:limestone unsupported backup persistent format version: "
"see https://github.com/project-tsurugi/tsurugidb/blob/master/docs/upgrade-guide.md";

status datastore::restore(std::string_view from, bool keep_backup) const noexcept {
Expand All @@ -36,15 +36,21 @@ status datastore::restore(std::string_view from, bool keep_backup) const noexcep
// log_dir version check
boost::filesystem::path manifest_path = from_dir / std::string(internal::manifest_file_name);
if (!boost::filesystem::exists(manifest_path)) {
LOG_LP(INFO) << "no manifest file in backup";
LOG(ERROR) << version_error_prefix << " (format version mismatch: version 0)";
VLOG_LP(log_info) << "no manifest file in backup";
LOG(ERROR) << version_error_prefix << " (version mismatch: version 0, server supports version 1)";
return status::err_broken_data;
}
std::string ver_err;
if (!internal::is_supported_version(manifest_path, ver_err)) { // notfound or invalid or unsupport
int vc = internal::is_supported_version(manifest_path, ver_err);
if (vc == 0) {
LOG(ERROR) << version_error_prefix << " (" << ver_err << ")";
return status::err_broken_data;
}
if (vc < 0) {
VLOG_LP(log_info) << ver_err;
LOG(ERROR) << "/:limestone backup data is corrupted, can not use.";
return status::err_broken_data;
}

BOOST_FOREACH(const boost::filesystem::path& p, std::make_pair(boost::filesystem::directory_iterator(location_), boost::filesystem::directory_iterator())) {
if(!boost::filesystem::is_directory(p)) {
Expand Down Expand Up @@ -101,15 +107,21 @@ status datastore::restore(std::string_view from, std::vector<file_set_entry>& en
return status::err_not_found;
}
std::string ver_err;
if (!internal::is_supported_version(src, ver_err)) {
int vc = internal::is_supported_version(src, ver_err);
if (vc == 0) {
LOG(ERROR) << version_error_prefix << " (" << ver_err << ")";
return status::err_broken_data;
}
if (vc < 0) {
VLOG_LP(log_info) << ver_err;
LOG(ERROR) << "/:limestone backup data is corrupted, can not use.";
return status::err_broken_data;
}
manifest_count++;
}
if (manifest_count < 1) { // XXX: change to != 1 ??
LOG_LP(INFO) << "no manifest file in backup";
LOG(ERROR) << version_error_prefix << " (format version mismatch: version 0)";
VLOG_LP(log_info) << "no manifest file in backup";
LOG(ERROR) << version_error_prefix << " (version mismatch: version 0, server supports version 1)";
return status::err_broken_data;
}

Expand Down
2 changes: 1 addition & 1 deletion src/limestone/internal.h
View file
Expand Up @@ -30,7 +30,7 @@ inline constexpr const std::string_view manifest_file_name = "limestone-manifest

void setup_initial_logdir(const boost::filesystem::path& logdir);

bool is_supported_version(const boost::filesystem::path& manifest_path, std::string& errmsg);
int is_supported_version(const boost::filesystem::path& manifest_path, std::string& errmsg);

void check_logdir_format(const boost::filesystem::path& logdir);

Expand Down
37 changes: 35 additions & 2 deletions test/limestone/log/log_dir_test.cpp
View file
Expand Up @@ -97,8 +97,7 @@ TEST_F(log_dir_test, accept_directory_only_correct_manifest_file) {
}

TEST_F(log_dir_test, reject_directory_of_different_version) {
create_file(boost::filesystem::path(location) / std::string(limestone::internal::manifest_file_name),
"{ \"format_version\": \"1.0\", \"persistent_format_version\": 222 }");
create_file(manifest_path, "{ \"format_version\": \"1.0\", \"persistent_format_version\": 222 }");

gen_datastore();
EXPECT_THROW({ limestone::internal::check_logdir_format(location); }, std::exception);
Expand Down Expand Up @@ -147,6 +146,21 @@ TEST_F(log_dir_test, rotate_old_rejects_v0_logdir_missing_manifest) {
EXPECT_EQ(datastore_->restore(bk_path.string(), true), limestone::status::err_broken_data);
}

TEST_F(log_dir_test, rotate_old_rejects_corrupted_dir) {
// setup backups
boost::filesystem::path bk_path = boost::filesystem::path(location) / "bk";
if (!boost::filesystem::create_directory(bk_path)) {
LOG(FATAL) << "cannot make directory";
}
create_file(bk_path / "epoch", "\x04\x00\x00\x00\x00\x00\x00\x00\x00");
create_file(bk_path / std::string(limestone::internal::manifest_file_name),
"{ \"answer\": 42 }");

gen_datastore();

EXPECT_EQ(datastore_->restore(bk_path.string(), true), limestone::status::err_broken_data);
}

TEST_F(log_dir_test, rotate_prusik_ok_v1_dir) {
// setup backups
boost::filesystem::path bk_path = boost::filesystem::path(location) / "bk";
Expand Down Expand Up @@ -201,4 +215,23 @@ TEST_F(log_dir_test, rotate_prusik_rejects_v0_logdir_missing_manifest) {
EXPECT_EQ(datastore_->restore(bk_path.string(), entries), limestone::status::err_broken_data);
}

TEST_F(log_dir_test, rotate_prusik_rejects_corrupted_dir) {
// setup backups
boost::filesystem::path bk_path = boost::filesystem::path(location) / "bk";
if (!boost::filesystem::create_directory(bk_path)) {
LOG(FATAL) << "cannot make directory";
}
create_file(bk_path / "epoch", "\x04\x00\x00\x00\x00\x00\x00\x00\x00");
create_file(bk_path / std::string(limestone::internal::manifest_file_name),
"{ \"answer\": 42 }");
// setup entries
std::vector<limestone::api::file_set_entry> entries;
entries.emplace_back("epoch", "epoch", false);
entries.emplace_back(std::string(limestone::internal::manifest_file_name), std::string(limestone::internal::manifest_file_name), false);

gen_datastore();

EXPECT_EQ(datastore_->restore(bk_path.string(), entries), limestone::status::err_broken_data);
}

} // namespace limestone::testing

0 comments on commit 4847e6b

Please sign in to comment.