Skip to content

Commit

Permalink
Refactor and add unit tests for online compaction functionality
Browse files Browse the repository at this point in the history
- Moved existing code to separate .cpp and .h files for better testability.
- Added unit tests for online compaction-related functions.
  • Loading branch information
umegane committed Aug 28, 2024
1 parent 09a1969 commit afd9e34
Show file tree
Hide file tree
Showing 4 changed files with 810 additions and 114 deletions.
82 changes: 3 additions & 79 deletions src/limestone/datastore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <limestone/api/compaction_catalog.h>
#include <limestone/api/rotation_task.h>
#include "log_entry.h"
#include "online_compaction.h"

namespace limestone::api {
using namespace limestone::internal;
Expand Down Expand Up @@ -444,6 +445,8 @@ void datastore::online_compaction_worker() {
boost::filesystem::path ctrl_dir = location_ / "ctrl";
boost::filesystem::path start_file = ctrl_dir / "start_compaction";


ensure_directory_exists(ctrl_dir);
if (!boost::filesystem::exists(ctrl_dir)) {
if (!boost::filesystem::create_directory(ctrl_dir)) {
VLOG(log_error) << "failed to create directory: " << ctrl_dir.string();
Expand Down Expand Up @@ -476,84 +479,6 @@ void datastore::stop_online_compaction_worker() {
cv_online_compaction_worker_.notify_all();
}

// TODO: move this function to a separate file
static void safe_rename(const boost::filesystem::path& from, const boost::filesystem::path& to) {
boost::system::error_code error;
boost::filesystem::rename(from, to, error);
if (error) {
LOG_LP(ERROR) << "fail to rename file: error_code: " << error << ", from: " << from << ", to: " << to;
throw std::runtime_error("fail to rename the file");
}
}



static std::set<std::string> select_files_for_compaction(const std::set<boost::filesystem::path>& rotation_end_files, std::set<std::string>& detached_pwals) {
std::set<std::string> need_compaction_filenames;
for (const boost::filesystem::path& path : rotation_end_files) {
std::string filename = path.filename().string();
if (filename.substr(0, 4) == "pwal" && filename.length() > 9 && detached_pwals.find(filename) == detached_pwals.end()) {
need_compaction_filenames.insert(filename);
detached_pwals.insert(filename);
LOG_LP(INFO) << "Selected file for compaction: " << filename;
} else {
LOG_LP(INFO) << "File skipped for compaction: " << filename << " (Reason: "
<< (filename.substr(0, 4) != "pwal" ? "does not start with 'pwal'" :
filename.length() <= 9 ? "filename length is 9 or less" :
"file is already detached") << ")";
}
}
return need_compaction_filenames;
}

static void ensure_directory_exists(const boost::filesystem::path& dir) {
if (boost::filesystem::exists(dir)) {
if (!boost::filesystem::is_directory(dir)) {
LOG_LP(ERROR) << "the path exists but is not a directory: " << dir;
throw std::runtime_error("The path exists but is not a directory: " + dir.string());
}
} else {
boost::system::error_code error;
const bool result_mkdir = boost::filesystem::create_directory(dir, error);
if (!result_mkdir || error) {
LOG_LP(ERROR) << "failed to create directory: result_mkdir: " << result_mkdir << ", error_code: " << error << ", path: " << dir;
throw std::runtime_error("Failed to create the directory");
}
}
}

static void handle_existing_compacted_file(const boost::filesystem::path& location) {
boost::filesystem::path compacted_file = location / compaction_catalog::get_compacted_filename();
boost::filesystem::path compacted_prev_file = location / compaction_catalog::get_compacted_backup_filename();

if (boost::filesystem::exists(compacted_file)) {
if (boost::filesystem::exists(compacted_prev_file)) {
LOG_LP(ERROR) << "the file already exists: " << compacted_prev_file;
throw std::runtime_error("The file already exists: " + compacted_prev_file.string());
}
safe_rename(compacted_file, compacted_prev_file);
}
}

static std::set<std::string> get_files_in_directory(const boost::filesystem::path& directory) {
std::set<std::string> files;
for (const auto& entry : boost::filesystem::directory_iterator(directory)) {
if (boost::filesystem::is_regular_file(entry.path())) {
files.insert(entry.path().filename().string());
}
}
return files;
}

static void remove_file_safely(const boost::filesystem::path& file) {
boost::system::error_code error;
boost::filesystem::remove(file, error);
if (error) {
LOG_LP(ERROR) << "failed to remove file: error_code: " << error << ", path: " << file;
throw std::runtime_error("Failed to remove the file");
}
}

void datastore::compact_with_online() {
// rotate first
rotation_result result = rotate_log_files();
Expand All @@ -575,7 +500,6 @@ void datastore::compact_with_online() {
return;
}


for (const auto& filename : need_compaction_filenames) {
LOG_LP(INFO) << "need_compaction_filenames: " << filename;
}
Expand Down
108 changes: 108 additions & 0 deletions src/limestone/online_compaction.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
#include "online_compaction.h"

#include <glog/logging.h>
#include <limestone/api/compaction_catalog.h>
#include <limestone/logging.h>

#include <boost/filesystem.hpp>
#include "logging_helper.h"

namespace limestone::internal {

using namespace limestone::api;

void safe_rename(const boost::filesystem::path& from, const boost::filesystem::path& to) {
boost::system::error_code error;
boost::filesystem::rename(from, to, error);
if (error) {
LOG_LP(ERROR) << "fail to rename file: error_code: " << error << ", from: " << from << ", to: " << to;
throw std::runtime_error("fail to rename the file");
}
}

std::set<std::string> select_files_for_compaction(const std::set<boost::filesystem::path>& rotation_end_files, std::set<std::string>& detached_pwals) {
std::set<std::string> need_compaction_filenames;
for (const boost::filesystem::path& path : rotation_end_files) {
std::string filename = path.filename().string();
if (filename.substr(0, 4) == "pwal" && filename.length() > 9 && detached_pwals.find(filename) == detached_pwals.end()) {
need_compaction_filenames.insert(filename);
detached_pwals.insert(filename);
LOG_LP(INFO) << "Selected file for compaction: " << filename;
} else {
LOG_LP(INFO) << "File skipped for compaction: " << filename << " (Reason: "
<< (filename.substr(0, 4) != "pwal" ? "does not start with 'pwal'" :
filename.length() <= 9 ? "filename length is 9 or less" :
"file is already detached") << ")";
}
}
return need_compaction_filenames;
}

void ensure_directory_exists(const boost::filesystem::path& dir) {
if (boost::filesystem::exists(dir)) {
if (!boost::filesystem::is_directory(dir)) {
LOG_LP(ERROR) << "the path exists but is not a directory: " << dir;
throw std::runtime_error("The path exists but is not a directory: " + dir.string());
}
} else {
boost::system::error_code error;
const bool result_mkdir = boost::filesystem::create_directory(dir, error);
if (!result_mkdir || error) {
LOG_LP(ERROR) << "failed to create directory: result_mkdir: " << result_mkdir << ", error_code: " << error << ", path: " << dir;
throw std::runtime_error("Failed to create the directory");
}
}
}

void handle_existing_compacted_file(const boost::filesystem::path& location) {
boost::filesystem::path compacted_file = location / compaction_catalog::get_compacted_filename();
boost::filesystem::path compacted_prev_file = location / compaction_catalog::get_compacted_backup_filename();

if (boost::filesystem::exists(compacted_file)) {
if (boost::filesystem::exists(compacted_prev_file)) {
LOG_LP(ERROR) << "the file already exists: " << compacted_prev_file;
throw std::runtime_error("The file already exists: " + compacted_prev_file.string());
}
safe_rename(compacted_file, compacted_prev_file);
}
}

std::set<std::string> get_files_in_directory(const boost::filesystem::path& directory) {
std::set<std::string> files;
boost::system::error_code error;

if (!boost::filesystem::exists(directory, error)) {
LOG_LP(ERROR) << "Directory does not exist: " << directory << ", error_code: " << error.message();
throw std::runtime_error("Directory does not exist: " + directory.string());
}

if (!boost::filesystem::is_directory(directory, error)) {
LOG_LP(ERROR) << "The path exists but is not a directory: " << directory << ", error_code: " << error.message();
throw std::runtime_error("The path exists but is not a directory: " + directory.string());
}

for (boost::filesystem::directory_iterator it(directory, error), end; it != end && !error; it.increment(error)) {
if (boost::filesystem::is_regular_file(it->path(), error)) {
files.insert(it->path().filename().string());
}
}

if (error) {
LOG_LP(ERROR) << "Error while iterating directory: " << directory << ", error_code: " << error.message();
throw std::runtime_error("Error while iterating directory: " + directory.string());
}

return files;
}


void remove_file_safely(const boost::filesystem::path& file) {
boost::system::error_code error;
boost::filesystem::remove(file, error);
if (error) {
LOG_LP(ERROR) << "failed to remove file: error_code: " << error << ", path: " << file;
throw std::runtime_error("Failed to remove the file");
}
}

}
83 changes: 83 additions & 0 deletions src/limestone/online_compaction.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#ifndef ONLINE_COMPACTION_H
#define ONLINE_COMPACTION_H

#include <boost/filesystem.hpp>
#include <set>
#include <string>

namespace limestone::internal {

/**
* @brief Safely renames a file or directory.
*
* This function attempts to rename a file or directory from one path to another.
* If the operation fails, it logs an error message and throws a runtime exception.
*
* @param from The current path of the file or directory.
* @param to The new path to rename the file or directory to.
* @throws std::runtime_error if the renaming operation fails.
*/
void safe_rename(const boost::filesystem::path& from, const boost::filesystem::path& to);

/**
* @brief Selects files for compaction based on specific criteria.
*
* This function iterates through a set of files that have reached the end of their rotation
* and selects those that are eligible for compaction. It adds selected files to a list and
* updates a set of detached files.
*
* @param rotation_end_files A set of file paths that have reached the end of their rotation.
* @param detached_pwals A set of filenames that are already detached and should not be selected.
* @return A set of filenames that have been selected for compaction.
*/
std::set<std::string> select_files_for_compaction(const std::set<boost::filesystem::path>& rotation_end_files, std::set<std::string>& detached_pwals);

/**
* @brief Ensures that a directory exists, creating it if necessary.
*
* This function checks if a directory exists at the specified path. If it does not exist,
* the function attempts to create it. If the path exists but is not a directory, or if
* directory creation fails, the function logs an error and throws an exception.
*
* @param dir The path of the directory to check or create.
* @throws std::runtime_error if the path exists but is not a directory, or if directory creation fails.
*/
void ensure_directory_exists(const boost::filesystem::path& dir);

/**
* @brief Handles an existing compacted file by renaming it if necessary.
*
* This function checks for the existence of a compacted file in a specified location. If a compacted
* file already exists and a backup file does not, the function renames the compacted file to a backup
* name. If both files exist, it logs an error and throws an exception.
*
* @param location The directory path where the compacted file is located.
* @throws std::runtime_error if both the compacted file and backup file already exist.
*/
void handle_existing_compacted_file(const boost::filesystem::path& location);

/**
* @brief Retrieves a list of all regular files in a specified directory.
*
* This function iterates over all entries in a given directory and returns a set containing
* the names of all regular files found.
*
* @param directory The path of the directory to search.
* @return A set of strings representing the names of all regular files in the directory.
*/
std::set<std::string> get_files_in_directory(const boost::filesystem::path& directory);

/**
* @brief Safely removes a file.
*
* This function attempts to remove a file at the specified path. If the removal fails, it logs
* an error message and throws a runtime exception.
*
* @param file The path of the file to be removed.
* @throws std::runtime_error if the file removal operation fails.
*/
void remove_file_safely(const boost::filesystem::path& file);

} // namespace limestone::internal

#endif // ONLINE_COMPACTION_H
Loading

0 comments on commit afd9e34

Please sign in to comment.