Skip to content

Commit

Permalink
Add handling of I/O exceptions during file rotation
Browse files Browse the repository at this point in the history
  • Loading branch information
umegane committed Sep 25, 2024
1 parent e316186 commit 41e2bdb
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/limestone/log_channel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ rotation_result log_channel::do_rotate_file(epoch_id_type epoch) {
<< "." << epoch;
std::string new_name = ss.str();
boost::filesystem::path new_file = location_ / new_name;
boost::filesystem::rename(file_path(), new_file);
boost::system::error_code ec;
boost::filesystem::rename(file_path(), new_file, ec);
if (ec) {
std::string err_msg = "Failed to rename file from " + file_path().string() + " to " + new_file.string() + ". Error: " + ec.message();
LOG_AND_THROW_IO_EXCEPTION(err_msg, ec);
}
envelope_.add_file(new_file);

registered_ = false;
Expand Down
1 change: 0 additions & 1 deletion src/limestone/rotation_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ void rotation_task::rotate() {
result_promise_.set_value(final_result);
} catch (const std::exception& e) {
auto ex_ptr = std::current_exception();
std::cerr << "Setting exception in promise" << std::endl;
result_promise_.set_exception(ex_ptr);
}
}
Expand Down
38 changes: 38 additions & 0 deletions test/limestone/compaction/online_compaction_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class online_compaction_test : public ::testing::Test {
const std::string compacted_filename = compaction_catalog::get_compacted_filename();

void SetUp() {
if (boost::filesystem::exists(location)) {
boost::filesystem::permissions(location, boost::filesystem::owner_all);
}
boost::filesystem::remove_all(location);
if (!boost::filesystem::create_directory(location)) {
std::cerr << "cannot make directory" << std::endl;
Expand All @@ -72,6 +75,9 @@ class online_compaction_test : public ::testing::Test {

void TearDown() {
datastore_ = nullptr;
if (boost::filesystem::exists(location)) {
boost::filesystem::permissions(location, boost::filesystem::owner_all);
}
boost::filesystem::remove_all(location);
}

Expand Down Expand Up @@ -820,6 +826,38 @@ TEST_F(online_compaction_test, scenario02) {
ASSERT_PRED_FORMAT3(ContainsPrefix, pwals, "pwal_0002.", 1);
}


TEST_F(online_compaction_test, fail_compact_with_io_error) {
gen_datastore();
datastore_->switch_epoch(1);
auto pwals = extract_pwal_files_from_datastore();
EXPECT_TRUE(pwals.empty());

lc0_->begin_session();
lc0_->add_entry(1, "k1", "v1", {1, 0});
lc0_->end_session();
lc1_->begin_session();
lc1_->add_entry(1, "k2", "v3", {1, 0});
lc1_->end_session();

compaction_catalog catalog = compaction_catalog::from_catalog_file(location);
EXPECT_EQ(catalog.get_max_epoch_id(), 0);
EXPECT_EQ(catalog.get_compacted_files().size(), 0);
EXPECT_EQ(catalog.get_detached_pwals().size(), 0);

pwals = extract_pwal_files_from_datastore();
EXPECT_EQ(pwals.size(), 2);
ASSERT_PRED_FORMAT2(ContainsString, pwals, "pwal_0000");
ASSERT_PRED_FORMAT2(ContainsString, pwals, "pwal_0001");

// remove the file to cause an I/O error
[[maybe_unused]] int result = std::system(("chmod 0500 " + std::string(location)).c_str());

// First compaction.
ASSERT_THROW(run_compact_with_epoch_switch(2), limestone_exception);

Check failure on line 857 in test/limestone/compaction/online_compaction_test.cpp

View workflow job for this annotation

GitHub Actions / CTest (ubuntu-22.04)

online_compaction_test.fail_compact_with_io_error

Expected: run_compact_with_epoch_switch(2) throws an exception of type limestone_exception. Actual: it throws nothing.

Check failure on line 857 in test/limestone/compaction/online_compaction_test.cpp

View workflow job for this annotation

GitHub Actions / CTest (ubuntu-24.04)

online_compaction_test.fail_compact_with_io_error

Expected: run_compact_with_epoch_switch(2) throws an exception of type limestone_exception. Actual: it throws nothing.
}


TEST_F(online_compaction_test, safe_rename_success) {
boost::filesystem::path from = boost::filesystem::path(location) / "test_file.txt";
boost::filesystem::path to = boost::filesystem::path(location) / "renamed_file.txt";
Expand Down

0 comments on commit 41e2bdb

Please sign in to comment.