Skip to content

Fix to bug that synced logs in the log file 0 are lost #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 11, 2025
Merged

Fix to bug that synced logs in the log file 0 are lost #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/log_mgr.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ Status LogMgr::init(const LogMgrOptions& lm_opt) {
}

try {
// WARNING:
// This flag should be set to `false` before doing initialization or load.
// For the first time creation, the very first log file will be created,
// and it should be synced to the disk. `addNewLogFile()` will set this
// flag to `true`. It should not be set to `false` after that.
needSkippedManiSync = false;

std::string m_filename;
if (opt.dbConfig->customManifestPath.empty()) {
// Normal open: manifest file in the same path.
Expand Down Expand Up @@ -145,6 +152,7 @@ Status LogMgr::init(const LogMgrOptions& lm_opt) {
l_file->setSyncedSeqNum(opt.startSeqnum - 1);
}
TC(mani->addNewLogFile(log_num, l_file, 1));
needSkippedManiSync = true;

} catch (Status s) {
delete l_file;
Expand Down Expand Up @@ -183,7 +191,6 @@ Status LogMgr::init(const LogMgrOptions& lm_opt) {
}
}

needSkippedManiSync = false;
initialized = true;
return Status();

Expand Down
92 changes: 68 additions & 24 deletions tests/jungle/log_reclaim_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,6 +1660,28 @@ int dedicated_flusher_test() {
return 0;
}

int insert_keys(jungle::DB* db, size_t from, size_t to) {
for (size_t ii = from; ii < to; ++ii) {
std::string key_str = "key" + TestSuite::lzStr(5, ii);
std::string val_str = "val" + TestSuite::lzStr(5, ii);
CHK_Z(db->set(jungle::KV(key_str, val_str)));
}
return 0;
}

int verify(jungle::DB* db, size_t upto) {
for (size_t ii = 0; ii < upto; ++ii) {
TestSuite::setInfo("ii=%zu", ii);
jungle::SizedBuf value_out;
jungle::SizedBuf::Holder h(value_out);
std::string key_str = "key" + TestSuite::lzStr(5, ii);
std::string val_str = "val" + TestSuite::lzStr(5, ii);
CHK_Z(db->get(jungle::SizedBuf(key_str), value_out));
CHK_EQ(val_str, value_out.toString());
}
return 0;
}

int sync_wo_manifest_test() {
std::string filename;
TEST_SUITE_PREPARE_PATH(filename);
Expand All @@ -1677,31 +1699,10 @@ int sync_wo_manifest_test() {
config.skipManifestSync = true;
CHK_Z(jungle::DB::open(&db, filename, config));

auto insert_keys = [&](size_t from, size_t to) {
for (size_t ii = from; ii < to; ++ii) {
std::string key_str = "key" + TestSuite::lzStr(5, ii);
std::string val_str = "val" + TestSuite::lzStr(5, ii);
CHK_Z( db->set( jungle::KV(key_str, val_str) ) );
}
return 0;
};
auto verify = [&](size_t upto) {
for (size_t ii = 0; ii < upto; ++ii) {
TestSuite::setInfo("ii=%zu", ii);
jungle::SizedBuf value_out;
jungle::SizedBuf::Holder h(value_out);
std::string key_str = "key" + TestSuite::lzStr(5, ii);
std::string val_str = "val" + TestSuite::lzStr(5, ii);
CHK_Z( db->get(jungle::SizedBuf(key_str), value_out) );
CHK_EQ(val_str, value_out.toString());
}
return 0;
};

CHK_Z(insert_keys(0, 11));
CHK_Z(insert_keys(db, 0, 11));
CHK_Z(db->sync(true));

CHK_Z(insert_keys(11, 15));
CHK_Z(insert_keys(db, 11, 15));
CHK_Z(db->sync(true));

// Copy file at this moment to mimic crash.
Expand All @@ -1713,7 +1714,47 @@ int sync_wo_manifest_test() {
// Open clone.
// Even with crash without manifest sync, all 15 logs should be there.
CHK_Z(jungle::DB::open(&db, clone_path, config));
CHK_Z(verify(15));
CHK_Z(verify(db, 15));
CHK_Z(jungle::DB::close(db));

CHK_Z(jungle::shutdown());

TEST_SUITE_CLEANUP_PATH();
return 0;
}

int sync_1st_file_wo_manifest_test() {
std::string filename;
TEST_SUITE_PREPARE_PATH(filename);

jungle::Status s;
jungle::DBConfig config;
TEST_CUSTOM_DB_CONFIG(config);
jungle::DB* db = nullptr;

jungle::GlobalConfig g_config;
g_config.numFlusherThreads = 1;
jungle::init(g_config);

config.maxEntriesInLogFile = 10;
config.skipManifestSync = true;
config.logSectionOnly = true;
CHK_Z(jungle::DB::open(&db, filename, config));

// Write on the first log file and sync.
CHK_Z(insert_keys(db, 0, 5));
CHK_Z(db->sync(true));

// Copy file at this moment to mimic crash.
std::string clone_path = filename + "_clone";
TestSuite::copyfile(filename, clone_path);

CHK_Z(jungle::DB::close(db));

// Open clone.
// Even with crash without manifest sync, all 5 logs should be there.
CHK_Z(jungle::DB::open(&db, clone_path, config));
CHK_Z(verify(db, 5));
CHK_Z(jungle::DB::close(db));

CHK_Z(jungle::shutdown());
Expand Down Expand Up @@ -1809,6 +1850,9 @@ int main(int argc, char** argv) {
ts.doTest("sync without manifest test",
sync_wo_manifest_test);

ts.doTest("sync 1st file without manifest test",
sync_1st_file_wo_manifest_test);

#if 0
ts.doTest("reload empty files test",
reload_with_empty_files_test_load);
Expand Down
Loading